Detect byte-swapped AC-3 (aka DNET) and support decoding it directly.

Message ID 4D80CA2C.5020208@gmail.com
State Superseded
Headers show

Commit Message

Justin Ruggles March 16, 2011, 2:33 p.m.
Patch by Reimar, originally sent to ffmpeg-devel@mplayerhq.hu

Comments

Mans Rullgard March 16, 2011, 2:40 p.m. | #1
Justin Ruggles <justin.ruggles@gmail.com> writes:

> Patch by Reimar, originally sent to ffmpeg-devel@mplayerhq.hu
>
>
> From b5f59c0b037e94393b8633c0e983594c4772de0d Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Reimar=20D=C3=B6ffinger?= <Reimar.Doeffinger@gmx.de>
> Date: Thu, 10 Mar 2011 22:38:35 +0100
> Subject: [PATCH 1/2] Detect byte-swapped AC-3 (aka DNET) and support decoding it directly.
> MIME-Version: 1.0
> Content-Type: multipart/mixed; boundary="------------1.7.1"
>
> This is a multi-part message in MIME format.
> --------------1.7.1
> Content-Type: text/plain; charset=UTF-8; format=fixed
> Content-Transfer-Encoding: 8bit
>
> This allows the AC-3 decoder to be used directly with RealMedia
> decoders that unlike the libavformat one do not byte-swap automatically.
> Since the new code is only used in case we would fail directly otherwise
> there should be no risk for regressions.
> The "buf" pointer needs to be overwritten since otherwise the CRC check fails.
> ---
>  libavcodec/ac3dec.c |   29 ++++++++++++++++++-----------
>  1 files changed, 18 insertions(+), 11 deletions(-)

Can we at least stop calling it dnet now?

> --------------1.7.1
> Content-Type: text/x-patch; name="0001-Detect-byte-swapped-AC-3-aka-DNET-and-support-decodi.patch"
> Content-Transfer-Encoding: 8bit
> Content-Disposition: attachment; filename="0001-Detect-byte-swapped-AC-3-aka-DNET-and-support-decodi.patch"
>
> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
> index 5ebee19..16e5176 100644
> --- a/libavcodec/ac3dec.c
> +++ b/libavcodec/ac3dec.c
> @@ -207,13 +207,6 @@ static av_cold int ac3_decode_init(AVCodecContext *avctx)
>      }
>      s->downmixed = 1;
>  
> -    /* allocate context input buffer */
> -    if (avctx->error_recognition >= FF_ER_CAREFUL) {
> -        s->input_buffer = av_mallocz(AC3_FRAME_BUFFER_SIZE + FF_INPUT_BUFFER_PADDING_SIZE);
> -        if (!s->input_buffer)
> -            return AVERROR(ENOMEM);
> -    }
> -
>      avctx->sample_fmt = AV_SAMPLE_FMT_S16;
>      return 0;
>  }
> @@ -1312,16 +1305,30 @@ static int ac3_decode_frame(AVCodecContext * avctx, void *data, int *data_size,
>      int blk, ch, err;
>      const uint8_t *channel_map;
>      const float *output[AC3_MAX_CHANNELS];
> +    // if it seems to be byte-swapped AC-3 (aka DNET)
> +    int is_swapped = buf_size >= 2 && AV_RB16(buf) == 0x770B;
>  
>      /* initialize the GetBitContext with the start of valid AC-3 Frame */
> -    if (s->input_buffer) {
> +    if (is_swapped || avctx->error_recognition >= FF_ER_CAREFUL) {
> +        /* allocate context input buffer */
> +        if (!s->input_buffer)
> +            s->input_buffer = av_mallocz(AC3_FRAME_BUFFER_SIZE + FF_INPUT_BUFFER_PADDING_SIZE);
> +        if (!s->input_buffer)
> +            return AVERROR(ENOMEM);
> +
>          /* copy input buffer to decoder context to avoid reading past the end
>             of the buffer, which can be caused by a damaged input stream. */
> +        if (is_swapped) {
> +            int i;
> +            for (i = 0; i < FFMIN(buf_size, AC3_FRAME_BUFFER_SIZE) - 1; i += 2) {
> +                s->input_buffer[i]     = buf[i + 1];
> +                s->input_buffer[i + 1] = buf[i];
> +            }
> +        } else

This should use the dsputil function I posted a patch for.

>          memcpy(s->input_buffer, buf, FFMIN(buf_size, AC3_FRAME_BUFFER_SIZE));
> -        init_get_bits(&s->gbc, s->input_buffer, buf_size * 8);
> -    } else {
> -        init_get_bits(&s->gbc, buf, buf_size * 8);
> +        buf = s->input_buffer;
>      }
> +    init_get_bits(&s->gbc, buf, buf_size * 8);
>  
>      /* parse the syncinfo */
>      *data_size = 0;
>
> --------------1.7.1--
>
>
Justin Ruggles March 16, 2011, 3:08 p.m. | #2
On 03/16/2011 10:40 AM, Måns Rullgård wrote:

> Justin Ruggles <justin.ruggles@gmail.com> writes:
> 
>> Patch by Reimar, originally sent to ffmpeg-devel@mplayerhq.hu
>>
>>
>> From b5f59c0b037e94393b8633c0e983594c4772de0d Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Reimar=20D=C3=B6ffinger?= <Reimar.Doeffinger@gmx.de>
>> Date: Thu, 10 Mar 2011 22:38:35 +0100
>> Subject: [PATCH 1/2] Detect byte-swapped AC-3 (aka DNET) and support decoding it directly.
>> MIME-Version: 1.0
>> Content-Type: multipart/mixed; boundary="------------1.7.1"
>>
>> This is a multi-part message in MIME format.
>> --------------1.7.1
>> Content-Type: text/plain; charset=UTF-8; format=fixed
>> Content-Transfer-Encoding: 8bit
>>
>> This allows the AC-3 decoder to be used directly with RealMedia
>> decoders that unlike the libavformat one do not byte-swap automatically.
>> Since the new code is only used in case we would fail directly otherwise
>> there should be no risk for regressions.
>> The "buf" pointer needs to be overwritten since otherwise the CRC check fails.
>> ---
>>  libavcodec/ac3dec.c |   29 ++++++++++++++++++-----------
>>  1 files changed, 18 insertions(+), 11 deletions(-)
> 
> Can we at least stop calling it dnet now?

Yes. Reimar already took out the comment calling it dnet, but I missed
that it was in the commit message.

>> --------------1.7.1
>> Content-Type: text/x-patch; name="0001-Detect-byte-swapped-AC-3-aka-DNET-and-support-decodi.patch"
>> Content-Transfer-Encoding: 8bit
>> Content-Disposition: attachment; filename="0001-Detect-byte-swapped-AC-3-aka-DNET-and-support-decodi.patch"
>>
>> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
>> index 5ebee19..16e5176 100644
>> --- a/libavcodec/ac3dec.c
>> +++ b/libavcodec/ac3dec.c
>> @@ -207,13 +207,6 @@ static av_cold int ac3_decode_init(AVCodecContext *avctx)
>>      }
>>      s->downmixed = 1;
>>  
>> -    /* allocate context input buffer */
>> -    if (avctx->error_recognition >= FF_ER_CAREFUL) {
>> -        s->input_buffer = av_mallocz(AC3_FRAME_BUFFER_SIZE + FF_INPUT_BUFFER_PADDING_SIZE);
>> -        if (!s->input_buffer)
>> -            return AVERROR(ENOMEM);
>> -    }
>> -
>>      avctx->sample_fmt = AV_SAMPLE_FMT_S16;
>>      return 0;
>>  }
>> @@ -1312,16 +1305,30 @@ static int ac3_decode_frame(AVCodecContext * avctx, void *data, int *data_size,
>>      int blk, ch, err;
>>      const uint8_t *channel_map;
>>      const float *output[AC3_MAX_CHANNELS];
>> +    // if it seems to be byte-swapped AC-3 (aka DNET)
>> +    int is_swapped = buf_size >= 2 && AV_RB16(buf) == 0x770B;
>>  
>>      /* initialize the GetBitContext with the start of valid AC-3 Frame */
>> -    if (s->input_buffer) {
>> +    if (is_swapped || avctx->error_recognition >= FF_ER_CAREFUL) {
>> +        /* allocate context input buffer */
>> +        if (!s->input_buffer)
>> +            s->input_buffer = av_mallocz(AC3_FRAME_BUFFER_SIZE + FF_INPUT_BUFFER_PADDING_SIZE);
>> +        if (!s->input_buffer)
>> +            return AVERROR(ENOMEM);
>> +
>>          /* copy input buffer to decoder context to avoid reading past the end
>>             of the buffer, which can be caused by a damaged input stream. */
>> +        if (is_swapped) {
>> +            int i;
>> +            for (i = 0; i < FFMIN(buf_size, AC3_FRAME_BUFFER_SIZE) - 1; i += 2) {
>> +                s->input_buffer[i]     = buf[i + 1];
>> +                s->input_buffer[i + 1] = buf[i];
>> +            }
>> +        } else
> 
> This should use the dsputil function I posted a patch for.

Indeed. Looks to be approved now, so I'll send a new patch once it's
committed.

-Justin
Mans Rullgard March 16, 2011, 3:31 p.m. | #3
Justin Ruggles <justin.ruggles@gmail.com> writes:

> On 03/16/2011 10:40 AM, Måns Rullgård wrote:
>
>> Justin Ruggles <justin.ruggles@gmail.com> writes:
>> 
>>> Patch by Reimar, originally sent to ffmpeg-devel@mplayerhq.hu
>>>
>>>
>>> From b5f59c0b037e94393b8633c0e983594c4772de0d Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Reimar=20D=C3=B6ffinger?= <Reimar.Doeffinger@gmx.de>
>>> Date: Thu, 10 Mar 2011 22:38:35 +0100
>>> Subject: [PATCH 1/2] Detect byte-swapped AC-3 (aka DNET) and support decoding it directly.
>>> MIME-Version: 1.0
>>> Content-Type: multipart/mixed; boundary="------------1.7.1"
>>>
>>> This is a multi-part message in MIME format.
>>> --------------1.7.1
>>> Content-Type: text/plain; charset=UTF-8; format=fixed
>>> Content-Transfer-Encoding: 8bit
>>>
>>> This allows the AC-3 decoder to be used directly with RealMedia
>>> decoders that unlike the libavformat one do not byte-swap automatically.
>>> Since the new code is only used in case we would fail directly otherwise
>>> there should be no risk for regressions.
>>> The "buf" pointer needs to be overwritten since otherwise the CRC check fails.
>>> ---
>>>  libavcodec/ac3dec.c |   29 ++++++++++++++++++-----------
>>>  1 files changed, 18 insertions(+), 11 deletions(-)
>> 
>> Can we at least stop calling it dnet now?
>
> Yes. Reimar already took out the comment calling it dnet, but I missed
> that it was in the commit message.
>
>>> @@ -1312,16 +1305,30 @@ static int ac3_decode_frame(AVCodecContext * avctx, void *data, int *data_size,
>>>      int blk, ch, err;
>>>      const uint8_t *channel_map;
>>>      const float *output[AC3_MAX_CHANNELS];
>>> +    // if it seems to be byte-swapped AC-3 (aka DNET)
                                                     ^^^^
It's still there ----------------------------------------

>>> +    int is_swapped = buf_size >= 2 && AV_RB16(buf) == 0x770B;
>>>  
>>>      /* initialize the GetBitContext with the start of valid AC-3 Frame */
>>> -    if (s->input_buffer) {
>>> +    if (is_swapped || avctx->error_recognition >= FF_ER_CAREFUL) {
>>> +        /* allocate context input buffer */
>>> +        if (!s->input_buffer)
>>> +            s->input_buffer = av_mallocz(AC3_FRAME_BUFFER_SIZE + FF_INPUT_BUFFER_PADDING_SIZE);
>>> +        if (!s->input_buffer)
>>> +            return AVERROR(ENOMEM);
>>> +
>>>          /* copy input buffer to decoder context to avoid reading past the end
>>>             of the buffer, which can be caused by a damaged input stream. */
>>> +        if (is_swapped) {
>>> +            int i;
>>> +            for (i = 0; i < FFMIN(buf_size, AC3_FRAME_BUFFER_SIZE) - 1; i += 2) {
>>> +                s->input_buffer[i]     = buf[i + 1];
>>> +                s->input_buffer[i + 1] = buf[i];
>>> +            }
>>> +        } else
>> 
>> This should use the dsputil function I posted a patch for.
>
> Indeed. Looks to be approved now, so I'll send a new patch once it's
> committed.

See what removing a few trolls can do...

Patch

From b5f59c0b037e94393b8633c0e983594c4772de0d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Reimar=20D=C3=B6ffinger?= <Reimar.Doeffinger@gmx.de>
Date: Thu, 10 Mar 2011 22:38:35 +0100
Subject: [PATCH 1/2] Detect byte-swapped AC-3 (aka DNET) and support decoding it directly.
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.7.1"

This is a multi-part message in MIME format.
--------------1.7.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


This allows the AC-3 decoder to be used directly with RealMedia
decoders that unlike the libavformat one do not byte-swap automatically.
Since the new code is only used in case we would fail directly otherwise
there should be no risk for regressions.
The "buf" pointer needs to be overwritten since otherwise the CRC check fails.
---
 libavcodec/ac3dec.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)


--------------1.7.1
Content-Type: text/x-patch; name="0001-Detect-byte-swapped-AC-3-aka-DNET-and-support-decodi.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Detect-byte-swapped-AC-3-aka-DNET-and-support-decodi.patch"

diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
index 5ebee19..16e5176 100644
--- a/libavcodec/ac3dec.c
+++ b/libavcodec/ac3dec.c
@@ -207,13 +207,6 @@  static av_cold int ac3_decode_init(AVCodecContext *avctx)
     }
     s->downmixed = 1;
 
-    /* allocate context input buffer */
-    if (avctx->error_recognition >= FF_ER_CAREFUL) {
-        s->input_buffer = av_mallocz(AC3_FRAME_BUFFER_SIZE + FF_INPUT_BUFFER_PADDING_SIZE);
-        if (!s->input_buffer)
-            return AVERROR(ENOMEM);
-    }
-
     avctx->sample_fmt = AV_SAMPLE_FMT_S16;
     return 0;
 }
@@ -1312,16 +1305,30 @@  static int ac3_decode_frame(AVCodecContext * avctx, void *data, int *data_size,
     int blk, ch, err;
     const uint8_t *channel_map;
     const float *output[AC3_MAX_CHANNELS];
+    // if it seems to be byte-swapped AC-3 (aka DNET)
+    int is_swapped = buf_size >= 2 && AV_RB16(buf) == 0x770B;
 
     /* initialize the GetBitContext with the start of valid AC-3 Frame */
-    if (s->input_buffer) {
+    if (is_swapped || avctx->error_recognition >= FF_ER_CAREFUL) {
+        /* allocate context input buffer */
+        if (!s->input_buffer)
+            s->input_buffer = av_mallocz(AC3_FRAME_BUFFER_SIZE + FF_INPUT_BUFFER_PADDING_SIZE);
+        if (!s->input_buffer)
+            return AVERROR(ENOMEM);
+
         /* copy input buffer to decoder context to avoid reading past the end
            of the buffer, which can be caused by a damaged input stream. */
+        if (is_swapped) {
+            int i;
+            for (i = 0; i < FFMIN(buf_size, AC3_FRAME_BUFFER_SIZE) - 1; i += 2) {
+                s->input_buffer[i]     = buf[i + 1];
+                s->input_buffer[i + 1] = buf[i];
+            }
+        } else
         memcpy(s->input_buffer, buf, FFMIN(buf_size, AC3_FRAME_BUFFER_SIZE));
-        init_get_bits(&s->gbc, s->input_buffer, buf_size * 8);
-    } else {
-        init_get_bits(&s->gbc, buf, buf_size * 8);
+        buf = s->input_buffer;
     }
+    init_get_bits(&s->gbc, buf, buf_size * 8);
 
     /* parse the syncinfo */
     *data_size = 0;

--------------1.7.1--