[libav-devel] [PATCH] [RFC] dcadec: don't check for overreads in auxiliary data.

[Tim Walker]

The auxiliary data length field is not reliable,
and incorrect overread errors could be returned
for valid, real-world bitstreams.
---
 libavcodec/dcadec.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/dcadec.c b/libavcodec/dcadec.c
index 610857d..7e94638 100644
--- a/libavcodec/dcadec.c
+++ b/libavcodec/dcadec.c
@@ -1086,12 +1086,12 @@ static int dca_subframe_footer(DCAContext *s, int 
base_channel)
 align_get_bits(>gb); // byte align
 skip_bits(>gb, 16);  // nAUXCRC16
 
-// additional data (reserved, cf. ETSI TS 102 114 V1.4.1)
-if ((reserved = (aux_data_end - get_bits_count(>gb))) < 0) {
-av_log(s->avctx, AV_LOG_ERROR,
-   "Overread auxiliary data by %d bits\n", -reserved);
-return AVERROR_INVALIDDATA;
-} else if (reserved) {
+/*
+ * additional data (reserved, cf. ETSI TS 102 114 V1.4.1)
+ *
+ * Note: don't check for overreads, aux_data_count can't be 
trusted.
+ */
+if ((reserved = (aux_data_end - get_bits_count(>gb))) > 0) {
 avpriv_request_sample(s->avctx,
   "Core auxiliary data reserved content");
 skip_bits_long(>gb, reserved);
-- 
2.4.9 (Apple Git-60)

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] [RFC] dcadec: don't check for overreads in auxiliary data.

[Tim W.]

On Sun, Nov 22, 2015 at 12:02 AM, Tim Walker  wrote:

> The auxiliary data length field is not reliable,
> and incorrect overread errors could be returned
> for valid, real-world bitstreams.
> ---
>  libavcodec/dcadec.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)


Been so long I had to use git send-email, I wanted to include more in the
initial email, sorry.

A sample is currently available at:
https://drive.google.com/file/d/0B814FrOKy8oHT0hVZ3RGNGdON2s/view?usp=sharing

This error seems to happen on several/many Blu-ray discs, too (try Googling
"Overread auxiliary data by 32 bits").

libdcadec simply ignores the field entirely:

libdcadec/core_decoder.c :
// Auxiliary data byte count (can't be trusted)
bits_skip(>bits, 6);

I'm unsure what the best approach is (mine or dcadec's), to be honest.
Thoughts?

Tim
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] [RFC] dcadec: don't check for overreads in auxiliary data.

[Luca Barbato]

On 22/11/15 00:11, Tim W. wrote:
> On Sun, Nov 22, 2015 at 12:02 AM, Tim Walker  wrote:
> 
>> The auxiliary data length field is not reliable,
>> and incorrect overread errors could be returned
>> for valid, real-world bitstreams.
>> ---
>>  libavcodec/dcadec.c | 12 ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> 
> Been so long I had to use git send-email, I wanted to include more in the
> initial email, sorry.
> 
> A sample is currently available at:
> https://drive.google.com/file/d/0B814FrOKy8oHT0hVZ3RGNGdON2s/view?usp=sharing
> 
> This error seems to happen on several/many Blu-ray discs, too (try Googling
> "Overread auxiliary data by 32 bits").
> 
> libdcadec simply ignores the field entirely:
> 
> libdcadec/core_decoder.c :
> // Auxiliary data byte count (can't be trusted)
> bits_skip(>bits, 6);
> 
> I'm unsure what the best approach is (mine or dcadec's), to be honest.
> Thoughts?
> 

Well assuming the user just gets the warning once it sounds fine, the
audio with your patch is ok?

I'm fine with your approach.

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] [RFC] dcadec: don't check for overreads in auxiliary data.

[Tim W.]

On Sun, Nov 22, 2015 at 12:30 AM, Luca Barbato  wrote:

>
>
> Well assuming the user just gets the warning once it sounds fine, the
> audio with your patch is ok?


AFAICT there is no discernible difference, the error only happens on one
frame and I really can't tell them apart.

I don't have any full-length Blu-ray discs on my hard drive right now,
there may or may not be discernible differences on some discs (though there
wasn't in the sample I tested before, but I unfortunately can't recall
which disc it was).

Given how widespread the issue is (see Google, quite a few hits), I'm
fairly confident libdcadec is right and aux_data_count can't be trusted.

I might still be OK to print a warning, but I think returning
AVERROR_INVALIDDATA is wrong, because this happens with many valid DTS
streams.

Tim
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel