| Message ID | 20240609150553.72865-1-ffmpeg@haasn.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/8] avdovi/dovi_rpudec: handle prev_vdr_rpu_id failures | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Sun, 09 Jun 2024 17:05:46 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote: > From: Niklas Haas <git@haasn.dev> > > According to the spec, missing previous VDR RPU IDs do not constitute an > error, but we should instead fallback first to VDR RPU with ID 0, and > failing that, synthesize "neutral" metadata. > > That's nontrivial though as the resulting metadata will be dependent on > other properties of the RPU, and this case is not hit in practice so > I'll defer it to a rainy day. > --- > libavcodec/dovi_rpudec.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c > index 7c7eda9d09..d1dcc3a262 100644 > --- a/libavcodec/dovi_rpudec.c > +++ b/libavcodec/dovi_rpudec.c > @@ -444,7 +444,12 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, > if (use_prev_vdr_rpu) { > int prev_vdr_rpu_id = get_ue_golomb_31(gb); > VALIDATE(prev_vdr_rpu_id, 0, DOVI_MAX_DM_ID); > + if (!s->vdr[prev_vdr_rpu_id]) > + prev_vdr_rpu_id = 0; > if (!s->vdr[prev_vdr_rpu_id]) { > + /* FIXME: Technically, the spec says that in this case we should > + * synthesize "neutral" vdr metadata, but easier to just error > + * out as this corner case is not hit in practice */ > av_log(s->logctx, AV_LOG_ERROR, "Unknown previous RPU ID: %u\n", > prev_vdr_rpu_id); > goto fail; > -- > 2.45.1 > Ping for review, otherwis will merge soon as it's a lot of relatively low-hanging fruit that fixes current deviations with the spec.
diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c index 7c7eda9d09..d1dcc3a262 100644 --- a/libavcodec/dovi_rpudec.c +++ b/libavcodec/dovi_rpudec.c @@ -444,7 +444,12 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, if (use_prev_vdr_rpu) { int prev_vdr_rpu_id = get_ue_golomb_31(gb); VALIDATE(prev_vdr_rpu_id, 0, DOVI_MAX_DM_ID); + if (!s->vdr[prev_vdr_rpu_id]) + prev_vdr_rpu_id = 0; if (!s->vdr[prev_vdr_rpu_id]) { + /* FIXME: Technically, the spec says that in this case we should + * synthesize "neutral" vdr metadata, but easier to just error + * out as this corner case is not hit in practice */ av_log(s->logctx, AV_LOG_ERROR, "Unknown previous RPU ID: %u\n", prev_vdr_rpu_id); goto fail;
From: Niklas Haas <git@haasn.dev> According to the spec, missing previous VDR RPU IDs do not constitute an error, but we should instead fallback first to VDR RPU with ID 0, and failing that, synthesize "neutral" metadata. That's nontrivial though as the resulting metadata will be dependent on other properties of the RPU, and this case is not hit in practice so I'll defer it to a rainy day. --- libavcodec/dovi_rpudec.c | 5 +++++ 1 file changed, 5 insertions(+)