* [FFmpeg-devel] [PATCH v2] avcodec/dovi_rpu: verify RPU data CRC32
@ 2023-08-09 17:05 quietvoid
2023-08-09 17:08 ` James Almer
0 siblings, 1 reply; 3+ messages in thread
From: quietvoid @ 2023-08-09 17:05 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: quietvoid
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);
+
+ 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");
--
2.41.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/dovi_rpu: verify RPU data CRC32
2023-08-09 17:05 [FFmpeg-devel] [PATCH v2] avcodec/dovi_rpu: verify RPU data CRC32 quietvoid
@ 2023-08-09 17:08 ` James Almer
2023-08-09 17:47 ` quietvoid
0 siblings, 1 reply; 3+ messages in thread
From: James Almer @ 2023-08-09 17:08 UTC (permalink / raw)
To: ffmpeg-devel
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");
_______________________________________________
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".
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/dovi_rpu: verify RPU data CRC32
2023-08-09 17:08 ` James Almer
@ 2023-08-09 17:47 ` quietvoid
0 siblings, 0 replies; 3+ messages in thread
From: quietvoid @ 2023-08-09 17:47 UTC (permalink / raw)
To: ffmpeg-devel
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
_______________________________________________
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".
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-09 17:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-09 17:05 [FFmpeg-devel] [PATCH v2] avcodec/dovi_rpu: verify RPU data CRC32 quietvoid
2023-08-09 17:08 ` James Almer
2023-08-09 17:47 ` quietvoid
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git