| Message ID | 20230809170500.24407-1-tcChlisop0@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2] avcodec/dovi_rpu: verify RPU data CRC32 | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 8/9/2023 2:05 PM, quietvoid wrote: > The Dolby Vision RPU contains a CRC32 to validate the payload against. > The implementation is CRC32/MPEG-2. > > The CRC is only verified with the AV_EF_CRCCHECK flag. > > Signed-off-by: quietvoid <tcChlisop0@gmail.com> > --- > libavcodec/dovi_rpu.c | 45 ++++++++++++++++++++++++++++++++++++++++--- > libavcodec/dovi_rpu.h | 3 ++- > libavcodec/hevcdec.c | 3 ++- > 3 files changed, 46 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c > index dd38936552..9ada0aceec 100644 > --- a/libavcodec/dovi_rpu.c > +++ b/libavcodec/dovi_rpu.c > @@ -22,6 +22,7 @@ > */ > > #include "libavutil/buffer.h" > +#include "libavutil/crc.h" > > #include "dovi_rpu.h" > #include "golomb.h" > @@ -191,13 +192,17 @@ static inline int64_t get_se_coef(GetBitContext *gb, const AVDOVIRpuDataHeader * > } \ > } while (0) > > -int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size) > +int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, > + int err_recognition) > { > AVDOVIRpuDataHeader *hdr = &s->header; > GetBitContext *gb = &(GetBitContext){0}; > DOVIVdrRef *vdr; > int ret; > > + size_t actual_rpu_size; > + uint8_t trailing_zeroes = 0; > + > uint8_t nal_prefix; > uint8_t rpu_type; > uint8_t vdr_seq_info_present; > @@ -205,7 +210,22 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size) > uint8_t use_prev_vdr_rpu; > uint8_t use_nlq; > uint8_t profile; > - if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0) > + > + uint32_t rpu_data_crc32; > + uint32_t computed_crc32; > + > + for (int i = rpu_size - 1; i > 0; i--) { > + if (!rpu[i]) { > + trailing_zeroes++; > + } else { > + break; > + } > + } > + > + actual_rpu_size = rpu_size - trailing_zeroes; > + > + /* Exclude trailing byte (0x80) from reader */ > + if ((ret = init_get_bits8(gb, rpu, actual_rpu_size - 1)) < 0) > return ret; > > /* RPU header, common values */ > @@ -440,7 +460,26 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size) > color->source_diagonal = get_bits(gb, 10); > } > > - /* FIXME: verify CRC32, requires implementation of AV_CRC_32_MPEG_2 */ > + /* Skip unsupported until CRC32 */ > + skip_bits_long(gb, get_bits_left(gb) - 32); This can be done after the err_recognition check. > + > + if (err_recognition & AV_EF_CRCCHECK) { Do instead if (!(err_recognition & AV_EF_CRCCHECK)) return 0; To remove one level of indentation. > + rpu_data_crc32 = get_bits_long(gb, 32); > + > + /* Verify CRC32, buffer excludes the prefix, CRC32 and trailing byte */ > + computed_crc32 = av_bswap32(av_crc(av_crc_get_table(AV_CRC_32_IEEE), > + -1, rpu + 1, actual_rpu_size - 6)); > + > + if (rpu_data_crc32 != computed_crc32) { > + av_log(s->logctx, AV_LOG_ERROR, > + "RPU CRC mismatch! Expected %"PRIu32", received %"PRIu32"\n", > + rpu_data_crc32, computed_crc32); > + > + if (err_recognition & AV_EF_EXPLODE) > + goto fail; > + } > + } > + > return 0; > > fail: > diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h > index f6ca5bbbc5..2b993a72c6 100644 > --- a/libavcodec/dovi_rpu.h > +++ b/libavcodec/dovi_rpu.h > @@ -77,7 +77,8 @@ void ff_dovi_update_cfg(DOVIContext *s, const AVDOVIDecoderConfigurationRecord * > * > * Returns 0 or an error code. > */ > -int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size); > +int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, > + int err_recognition); > > /** > * Attach the decoded AVDOVIMetadata as side data to an AVFrame. > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c > index df40c91ba6..81b1a84625 100644 > --- a/libavcodec/hevcdec.c > +++ b/libavcodec/hevcdec.c > @@ -3182,7 +3182,8 @@ static int decode_nal_units(HEVCContext *s, const uint8_t *buf, int length) > return AVERROR(ENOMEM); > memcpy(s->rpu_buf->data, nal->raw_data + 2, nal->raw_size - 2); > > - ret = ff_dovi_rpu_parse(&s->dovi_ctx, nal->data + 2, nal->size - 2); > + ret = ff_dovi_rpu_parse(&s->dovi_ctx, nal->data + 2, nal->size - 2, > + s->avctx->err_recognition); > if (ret < 0) { > av_buffer_unref(&s->rpu_buf); > av_log(s->avctx, AV_LOG_WARNING, "Error parsing DOVI NAL unit.\n");
On 09/08/2023 13.08, James Almer wrote: > On 8/9/2023 2:05 PM, quietvoid wrote: >> The Dolby Vision RPU contains a CRC32 to validate the payload against. >> The implementation is CRC32/MPEG-2. >> >> The CRC is only verified with the AV_EF_CRCCHECK flag. >> >> Signed-off-by: quietvoid <tcChlisop0@gmail.com> >> --- >> libavcodec/dovi_rpu.c | 45 ++++++++++++++++++++++++++++++++++++++++--- >> libavcodec/dovi_rpu.h | 3 ++- >> libavcodec/hevcdec.c | 3 ++- >> 3 files changed, 46 insertions(+), 5 deletions(-) >> >> diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c >> index dd38936552..9ada0aceec 100644 >> --- a/libavcodec/dovi_rpu.c >> +++ b/libavcodec/dovi_rpu.c >> @@ -22,6 +22,7 @@ >> */ >> #include "libavutil/buffer.h" >> +#include "libavutil/crc.h" >> #include "dovi_rpu.h" >> #include "golomb.h" >> @@ -191,13 +192,17 @@ static inline int64_t get_se_coef(GetBitContext >> *gb, const AVDOVIRpuDataHeader * >> } \ >> } while (0) >> -int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t >> rpu_size) >> +int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t >> rpu_size, >> + int err_recognition) >> { >> AVDOVIRpuDataHeader *hdr = &s->header; >> GetBitContext *gb = &(GetBitContext){0}; >> DOVIVdrRef *vdr; >> int ret; >> + size_t actual_rpu_size; >> + uint8_t trailing_zeroes = 0; >> + >> uint8_t nal_prefix; >> uint8_t rpu_type; >> uint8_t vdr_seq_info_present; >> @@ -205,7 +210,22 @@ int ff_dovi_rpu_parse(DOVIContext *s, const >> uint8_t *rpu, size_t rpu_size) >> uint8_t use_prev_vdr_rpu; >> uint8_t use_nlq; >> uint8_t profile; >> - if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0) >> + >> + uint32_t rpu_data_crc32; >> + uint32_t computed_crc32; >> + >> + for (int i = rpu_size - 1; i > 0; i--) { >> + if (!rpu[i]) { >> + trailing_zeroes++; >> + } else { >> + break; >> + } >> + } >> + >> + actual_rpu_size = rpu_size - trailing_zeroes; >> + >> + /* Exclude trailing byte (0x80) from reader */ >> + if ((ret = init_get_bits8(gb, rpu, actual_rpu_size - 1)) < 0) >> return ret; >> /* RPU header, common values */ >> @@ -440,7 +460,26 @@ int ff_dovi_rpu_parse(DOVIContext *s, const >> uint8_t *rpu, size_t rpu_size) >> color->source_diagonal = get_bits(gb, 10); >> } >> - /* FIXME: verify CRC32, requires implementation of >> AV_CRC_32_MPEG_2 */ >> + /* Skip unsupported until CRC32 */ >> + skip_bits_long(gb, get_bits_left(gb) - 32); > > This can be done after the err_recognition check. > >> + >> + if (err_recognition & AV_EF_CRCCHECK) { > > Do instead > > if (!(err_recognition & AV_EF_CRCCHECK)) > return 0; > > To remove one level of indentation. Thanks, sent v3: https://ffmpeg.org/pipermail/ffmpeg-devel/2023-August/313141.html
diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c index dd38936552..9ada0aceec 100644 --- a/libavcodec/dovi_rpu.c +++ b/libavcodec/dovi_rpu.c @@ -22,6 +22,7 @@ */ #include "libavutil/buffer.h" +#include "libavutil/crc.h" #include "dovi_rpu.h" #include "golomb.h" @@ -191,13 +192,17 @@ static inline int64_t get_se_coef(GetBitContext *gb, const AVDOVIRpuDataHeader * } \ } while (0) -int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size) +int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, + int err_recognition) { AVDOVIRpuDataHeader *hdr = &s->header; GetBitContext *gb = &(GetBitContext){0}; DOVIVdrRef *vdr; int ret; + size_t actual_rpu_size; + uint8_t trailing_zeroes = 0; + uint8_t nal_prefix; uint8_t rpu_type; uint8_t vdr_seq_info_present; @@ -205,7 +210,22 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size) uint8_t use_prev_vdr_rpu; uint8_t use_nlq; uint8_t profile; - if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0) + + uint32_t rpu_data_crc32; + uint32_t computed_crc32; + + for (int i = rpu_size - 1; i > 0; i--) { + if (!rpu[i]) { + trailing_zeroes++; + } else { + break; + } + } + + actual_rpu_size = rpu_size - trailing_zeroes; + + /* Exclude trailing byte (0x80) from reader */ + if ((ret = init_get_bits8(gb, rpu, actual_rpu_size - 1)) < 0) return ret; /* RPU header, common values */ @@ -440,7 +460,26 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size) color->source_diagonal = get_bits(gb, 10); } - /* FIXME: verify CRC32, requires implementation of AV_CRC_32_MPEG_2 */ + /* Skip unsupported until CRC32 */ + skip_bits_long(gb, get_bits_left(gb) - 32); + + if (err_recognition & AV_EF_CRCCHECK) { + rpu_data_crc32 = get_bits_long(gb, 32); + + /* Verify CRC32, buffer excludes the prefix, CRC32 and trailing byte */ + computed_crc32 = av_bswap32(av_crc(av_crc_get_table(AV_CRC_32_IEEE), + -1, rpu + 1, actual_rpu_size - 6)); + + if (rpu_data_crc32 != computed_crc32) { + av_log(s->logctx, AV_LOG_ERROR, + "RPU CRC mismatch! Expected %"PRIu32", received %"PRIu32"\n", + rpu_data_crc32, computed_crc32); + + if (err_recognition & AV_EF_EXPLODE) + goto fail; + } + } + return 0; fail: diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h index f6ca5bbbc5..2b993a72c6 100644 --- a/libavcodec/dovi_rpu.h +++ b/libavcodec/dovi_rpu.h @@ -77,7 +77,8 @@ void ff_dovi_update_cfg(DOVIContext *s, const AVDOVIDecoderConfigurationRecord * * * Returns 0 or an error code. */ -int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size); +int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, + int err_recognition); /** * Attach the decoded AVDOVIMetadata as side data to an AVFrame. diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c index df40c91ba6..81b1a84625 100644 --- a/libavcodec/hevcdec.c +++ b/libavcodec/hevcdec.c @@ -3182,7 +3182,8 @@ static int decode_nal_units(HEVCContext *s, const uint8_t *buf, int length) return AVERROR(ENOMEM); memcpy(s->rpu_buf->data, nal->raw_data + 2, nal->raw_size - 2); - ret = ff_dovi_rpu_parse(&s->dovi_ctx, nal->data + 2, nal->size - 2); + ret = ff_dovi_rpu_parse(&s->dovi_ctx, nal->data + 2, nal->size - 2, + s->avctx->err_recognition); if (ret < 0) { av_buffer_unref(&s->rpu_buf); av_log(s->avctx, AV_LOG_WARNING, "Error parsing DOVI NAL unit.\n");
The Dolby Vision RPU contains a CRC32 to validate the payload against. The implementation is CRC32/MPEG-2. The CRC is only verified with the AV_EF_CRCCHECK flag. Signed-off-by: quietvoid <tcChlisop0@gmail.com> --- libavcodec/dovi_rpu.c | 45 ++++++++++++++++++++++++++++++++++++++++--- libavcodec/dovi_rpu.h | 3 ++- libavcodec/hevcdec.c | 3 ++- 3 files changed, 46 insertions(+), 5 deletions(-)