| Message ID | 0101018f98b99e17-5074658e-5904-4d46-8d6f-4a7f3630bdd4-000000@us-west-2.amazonses.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/dovi_rpudec - correctly read el_bit_depth_minus8 when ext_mapping_idc is non-zero | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/commit_msg_loongarch64 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
Cosmin Stejerean via ffmpeg-devel: > From: Cosmin Stejerean <cosmin@cosmin.at> > > It looks like the el_bitdepth_minus8 value in the header can also encode > ext_mapping_idc in the upper 8 bits. > > Samples having a non-zero ext_mapping_idc fail validation currently because the > value returned is out of range. This bypasses this by currently ignoring the > ext_mapping_idc and using only the lower 8 bits for el_bitdepth_minus8. > > --- > libavcodec/dovi_rpudec.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c > index 7c7eda9d09..1b11ad3640 100644 > --- a/libavcodec/dovi_rpudec.c > +++ b/libavcodec/dovi_rpudec.c > @@ -411,7 +411,9 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, > > if ((hdr->rpu_format & 0x700) == 0) { > int bl_bit_depth_minus8 = get_ue_golomb_31(gb); > - int el_bit_depth_minus8 = get_ue_golomb_31(gb); > + // this can encode a two byte value, with higher byte being ext_mapping_idc > + // use only the lower byte for el_bit_depth_minus8 > + uint8_t el_bit_depth_minus8 = get_ue_golomb_long(gb) & 0xFF; > int vdr_bit_depth_minus8 = get_ue_golomb_31(gb); > VALIDATE(bl_bit_depth_minus8, 0, 8); > VALIDATE(el_bit_depth_minus8, 0, 8); So there is a field (ext_mapping_idc) in RPU that neither rpudec nor rpuenc (nor dovi_meta.h) know about? In this case, we should disable the encoding parts. - Andreas
On Tue, 21 May 2024 01:17:32 +0000 Cosmin Stejerean via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote: > From: Cosmin Stejerean <cosmin@cosmin.at> > > It looks like the el_bitdepth_minus8 value in the header can also encode > ext_mapping_idc in the upper 8 bits. > > Samples having a non-zero ext_mapping_idc fail validation currently because the > value returned is out of range. This bypasses this by currently ignoring the > ext_mapping_idc and using only the lower 8 bits for el_bitdepth_minus8. What is ext_mapping_idc? If it's signalled data that can't be reconstructed, we need to store it somewhere into AVDOVIMetadata and then re-synthesize it during encoding. Otherwise the RPU transcode will be lossy. > > --- > libavcodec/dovi_rpudec.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c > index 7c7eda9d09..1b11ad3640 100644 > --- a/libavcodec/dovi_rpudec.c > +++ b/libavcodec/dovi_rpudec.c > @@ -411,7 +411,9 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, > > if ((hdr->rpu_format & 0x700) == 0) { > int bl_bit_depth_minus8 = get_ue_golomb_31(gb); > - int el_bit_depth_minus8 = get_ue_golomb_31(gb); > + // this can encode a two byte value, with higher byte being ext_mapping_idc > + // use only the lower byte for el_bit_depth_minus8 > + uint8_t el_bit_depth_minus8 = get_ue_golomb_long(gb) & 0xFF; > int vdr_bit_depth_minus8 = get_ue_golomb_31(gb); > VALIDATE(bl_bit_depth_minus8, 0, 8); > VALIDATE(el_bit_depth_minus8, 0, 8); > -- > 2.42.1 > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> On May 21, 2024, at 3:21 AM, Niklas Haas <ffmpeg@haasn.xyz> wrote: > > On Tue, 21 May 2024 01:17:32 +0000 Cosmin Stejerean via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote: >> From: Cosmin Stejerean <cosmin@cosmin.at> >> >> It looks like the el_bitdepth_minus8 value in the header can also encode >> ext_mapping_idc in the upper 8 bits. >> >> Samples having a non-zero ext_mapping_idc fail validation currently because the >> value returned is out of range. This bypasses this by currently ignoring the >> ext_mapping_idc and using only the lower 8 bits for el_bitdepth_minus8. > > What is ext_mapping_idc? If it's signalled data that can't be > reconstructed, we need to store it somewhere into AVDOVIMetadata and > then re-synthesize it during encoding. Otherwise the RPU transcode will > be lossy. I'm not actually sure what it does, but from what I can tell on the current samples it doesn't matter if in the process of transcoding it ends up being set to 0. However it's not hard to save it and re-synthesize it so I can send a new patch that does that. - Cosmin
diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c index 7c7eda9d09..1b11ad3640 100644 --- a/libavcodec/dovi_rpudec.c +++ b/libavcodec/dovi_rpudec.c @@ -411,7 +411,9 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, if ((hdr->rpu_format & 0x700) == 0) { int bl_bit_depth_minus8 = get_ue_golomb_31(gb); - int el_bit_depth_minus8 = get_ue_golomb_31(gb); + // this can encode a two byte value, with higher byte being ext_mapping_idc + // use only the lower byte for el_bit_depth_minus8 + uint8_t el_bit_depth_minus8 = get_ue_golomb_long(gb) & 0xFF; int vdr_bit_depth_minus8 = get_ue_golomb_31(gb); VALIDATE(bl_bit_depth_minus8, 0, 8); VALIDATE(el_bit_depth_minus8, 0, 8);
From: Cosmin Stejerean <cosmin@cosmin.at> It looks like the el_bitdepth_minus8 value in the header can also encode ext_mapping_idc in the upper 8 bits. Samples having a non-zero ext_mapping_idc fail validation currently because the value returned is out of range. This bypasses this by currently ignoring the ext_mapping_idc and using only the lower 8 bits for el_bitdepth_minus8. --- libavcodec/dovi_rpudec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)