Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 29 additions & 23 deletions custom_components/xmeye/camera.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,18 +400,26 @@ async def async_added_to_hass(self) -> None:

async def async_will_remove_from_hass(self) -> None:
await super().async_will_remove_from_hass()
# Best-effort cleanup of any go2rtc stream we registered. Failures
# are logged at debug only — the entity removal must not block.
if self._go2rtc_stream_name is not None:
client: Go2RTCClient | None = self._coordinator.go2rtc_client
if client is not None:
try:
await client.unregister_stream(self._go2rtc_stream_name)
except Exception: # noqa: BLE001
_LOGGER.debug(
"go2rtc unregister failed for %s", self._go2rtc_stream_name
)
self._go2rtc_stream_name = None
await self._drop_go2rtc_stream()

async def _drop_go2rtc_stream(self) -> None:
"""Remove our go2rtc registration for this channel, if any.

Deleting unconditionally rather than only when we believe we
registered: an entry left behind by an earlier run would otherwise
survive forever, because HA's provider skips its rewrite whenever
producer[0] still matches the camera's stream source.

Best-effort throughout — entity removal must not block on go2rtc.
"""
client: Go2RTCClient | None = self._coordinator.go2rtc_client
if client is not None:
identifier = self._go2rtc_stream_name or _go2rtc_identifier(self)
try:
await client.unregister_stream(identifier)
except Exception: # noqa: BLE001
_LOGGER.debug("go2rtc unregister failed for %s", identifier)
self._go2rtc_stream_name = None
# Drop this channel from the device-wide H.265 advisory. Sibling
# channels keep theirs.
if self._coordinator.h265_channels.pop(self._channel, None) is not None:
Expand Down Expand Up @@ -454,12 +462,13 @@ async def _maybe_register_go2rtc(self) -> None:
if not options.get(CONF_TRANSCODE_H265, DEFAULT_TRANSCODE_H265):
_LOGGER.info(
"ch%d is H.265 but transcoding is disabled for this device — "
"leaving the stream to Home Assistant's defaults",
"handing the stream back to Home Assistant",
self._channel + 1,
)
# Registers nothing; called only so any advisory left over from
# before it was switched off gets cleared.
await self._ensure_go2rtc_stream(self._stream_url)
# Not registering is not enough: an entry from a run where it was
# still enabled would stay alive, because HA's provider skips its
# rewrite while producer[0] matches the stream source.
await self._drop_go2rtc_stream()
return
_LOGGER.info(
"ch%d is H.265 — registering with go2rtc (H.265 passthrough + "
Expand Down Expand Up @@ -496,16 +505,13 @@ async def _ensure_go2rtc_stream(self, url: str) -> bool:
and one of its producers matches ``await camera.stream_source()``.

Returns False without touching go2rtc when the user has turned
transcoding off for this device.
transcoding off for this device. Removing a leftover registration is
:meth:`_drop_go2rtc_stream`'s job, done once per entity add — this
method sits on the hot path (every WebRTC offer, every snapshot) and
must not issue HTTP just to discover it has nothing to do.
"""
options = self._coordinator.entry.options
if not options.get(CONF_TRANSCODE_H265, DEFAULT_TRANSCODE_H265):
# Deliberate choice by the user: register nothing and let HA
# handle the H.265 stream its own way. Dropping the channel also
# clears both advisories — nobody should be nagged about a
# trade-off they made on purpose.
if self._coordinator.h265_channels.pop(self._channel, None) is not None:
_refresh_h265_issues(self.hass, self._coordinator)
return False

client: Go2RTCClient | None = self._coordinator.go2rtc_client
Expand Down
1 change: 1 addition & 0 deletions custom_components/xmeye/diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ async def async_get_config_entry_diagnostics(
coordinator = entry.runtime_data
return {
"config": async_redact_data(dict(entry.data), TO_REDACT),
"options": dict(entry.options),
"connection": {
"connected": coordinator.connected,
"channel_count": coordinator.channel_count,
Expand Down
13 changes: 10 additions & 3 deletions custom_components/xmeye/go2rtc_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,20 @@ async def _put_stream(self, base: str, name: str, sources: list[str]) -> bool:
return False

async def unregister_stream(self, name: str) -> bool:
"""Remove a stream from go2rtc. Best-effort: failures are logged only."""
"""Remove a stream from go2rtc. Best-effort: failures are logged only.

DELETE is keyed off ``src``, not ``name`` — ``PUT`` is the verb that
takes ``name``. And a request that reaches ``/api/streams`` without a
``src`` short-circuits into "list every stream" and answers 200
(``internal/streams/api.go``), so getting this wrong is a no-op that
reports success. Hence the empty-name guard.
"""
base = await self._ensure()
if base is None or self._session is None:
if base is None or self._session is None or not name:
return False
try:
async with self._session.delete(
f"{base}{GO2RTC_STREAMS_PATH}", params={"name": name}, timeout=_TIMEOUT
f"{base}{GO2RTC_STREAMS_PATH}", params={"src": name}, timeout=_TIMEOUT
) as resp:
ok = resp.status < 300
if ok:
Expand Down
2 changes: 1 addition & 1 deletion custom_components/xmeye/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
"issue_tracker": "https://github.com/equake/hass-xmeye/issues",
"quality_scale": "silver",
"requirements": [],
"version": "0.8.2"
"version": "0.8.3"
}
72 changes: 72 additions & 0 deletions docs/STREAM_INVESTIGATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -335,3 +335,75 @@ conectado é
`rtsp://127.0.0.1:18554/<id>?video&audio&source=ffmpeg:<id>%23video%3Dh264%23audio%3Dopus…`,
que depois do `unquote` contém a string da fonte literalmente, sufixo incluído — verificado
em campo. Trocar a opção muda a string, o match falha de propósito, e o PUT reescreve.

---

## 11. Bug (v0.8.3) — o `unregister_stream` nunca removeu nada

O checkbox de transcoding adicionado na v0.8.2 não desligava coisa alguma: com ele
desmarcado, os três canais continuavam registrados com a nossa string
`ffmpeg:<id>#video=h264#audio=opus#bitrate=2048k`.

### Causa raiz

`internal/streams/api.go` do go2rtc:

```go
query := r.URL.Query()
src := query.Get("src")

// without source - return all streams list
if src == "" && r.Method != "POST" {
api.ResponseJSON(w, streams)
return
}

switch r.Method {
case "PUT":
name := query.Get("name") // PUT usa `name`
...
case "DELETE":
delete(streams, src) // DELETE usa `src` como chave (= o nome do stream)
```

O `unregister_stream` mandava `?name=`. Sem `src`, a requisição cai no early-return, devolve
**200 com a lista completa de streams** e o `case "DELETE"` nunca roda. Como o código testava
`resp.status < 300`, isso era tratado como sucesso e logado como "go2rtc stream
unregistered". Estava assim desde o PR #32 — nunca tinha sido exercitado porque nada
dependia da remoção até o toggle existir.

Comprovado contra a instância real, com o mesmo stream:

| Requisição | HTTP | Corpo | Stream depois |
|---|---|---|---|
| `DELETE /api/streams?name=<id>` | 200 | 1310 B (a lista inteira) | **continua lá** |
| `DELETE /api/streams?src=<id>` | 200 | 0 B | **removido** |

### Por que não se autocorrigia

Depois do teardown falho, com o transcoding desligado, nada mais registrava — mas o
`_update_stream_source` do HA só reescreve quando o stream não existe **ou** nenhum producer
bate com o `stream_source`. O stream velho existia e o producer[0] era exatamente o
`stream_source`, então o HA pulava a reescrita e a entrada obsoleta sobrevivia
indefinidamente. O mesmo mecanismo que faz a v0.8.0 funcionar é o que prendia o estado
errado aqui.

### Por que era difícil de enxergar

Os sinais naturais — "tem um `ffmpeg` rodando" e "o go2rtc mostra 2 online" — valem para os
dois estados do checkbox. O HA registra **sempre** dois sources para qualquer câmera, e o
segundo é sempre um `ffmpeg:` (o dele só transcodifica áudio, `#audio=opus#query=…`, sem
`#video=`). Confirmado na mesma instância com uma câmera Tuya, alheia a esta integração. O
que distingue os dois estados é o **conteúdo** do source[1], não a contagem.

### Correções

1. `unregister_stream` passa a usar `src=`, com guarda contra nome vazio (que reintroduziria
exatamente o mesmo no-op).
2. `_drop_go2rtc_stream()` novo, chamado tanto no teardown quanto no caminho
"transcoding desligado" do `_maybe_register_go2rtc`. Deleta incondicionalmente, sem
depender de `_go2rtc_stream_name`, para limpar sobras de execuções anteriores.
3. `_ensure_go2rtc_stream` volta a só registrar: no caminho desligado apenas `return False`,
sem HTTP — ele roda a cada oferta WebRTC e a cada snapshot.
4. `diagnostics.py` passa a incluir `entry.options`. A ausência disso foi o que impediu
responder "a opção foi mesmo salva?" sem pedir ao usuário para abrir o formulário.
Loading