diff mbox series

[FFmpeg-devel] IEC61937_EAC3 decoding support

Message ID 20210410081606.557527-1-Shulyaka@gmail.com
State New
Headers show
Series [FFmpeg-devel] IEC61937_EAC3 decoding support
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Denis Shulyaka April 10, 2021, 8:16 a.m. UTC
This patch adds support to decode IEC61937_EAC3 (aka dolby digital plus).

Signed-off-by: Denis Shulyaka <Shulyaka@gmail.com>
---
 libavformat/spdifdec.c | 47 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 9 deletions(-)

Comments

Carl Eugen Hoyos April 10, 2021, 3:04 p.m. UTC | #1
Am Sa., 10. Apr. 2021 um 10:46 Uhr schrieb Denis Shulyaka <shulyaka@gmail.com>:
>
> This patch adds support to decode IEC61937_EAC3 (aka dolby digital plus).
>
> Signed-off-by: Denis Shulyaka <Shulyaka@gmail.com>
> ---
>  libavformat/spdifdec.c | 47 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/libavformat/spdifdec.c b/libavformat/spdifdec.c
> index 1808fa9d65..58841e7775 100644
> --- a/libavformat/spdifdec.c
> +++ b/libavformat/spdifdec.c
> @@ -93,6 +93,10 @@ static int spdif_get_offset_and_codec(AVFormatContext *s,
>          *offset = 8192;
>          *codec = AV_CODEC_ID_DTS;
>          break;
> +    case IEC61937_EAC3:
> +        *offset = AC3_FRAME_SIZE << 4;
> +        *codec = AV_CODEC_ID_EAC3;
> +        break;
>      default:
>          if (s) { /* be silent during a probe */
>              avpriv_request_sample(s, "Data type 0x%04x in IEC 61937",
> @@ -103,9 +107,34 @@ static int spdif_get_offset_and_codec(AVFormatContext *s,
>      return 0;
>  }
>
> -/* Largest offset between bursts we currently handle, i.e. AAC with
> -   samples = 4096 */
> -#define SPDIF_MAX_OFFSET 16384
> +static int spdif_read_burst_payload_length(AVFormatContext *s,
> +                                           enum IEC61937DataType data_type)
> +{
> +    AVIOContext *pb = s->pb;
> +    int pkt_size_bits, pkt_size_bytes;
> +
> +    switch (data_type & 0xff) {
> +    case IEC61937_EAC3:
> +        pkt_size_bytes = avio_rl16(pb);
> +
> +        if (pkt_size_bytes % 2)

Please use "&" instead of "%".

> +            avpriv_request_sample(s, "Packet not ending at a 16-bit boundary");
> +
> +        return FFALIGN(pkt_size_bytes, 2);

> +        break;

Please remove this line.

> +    default:
> +        pkt_size_bits = avio_rl16(pb);
> +

> +        if (pkt_size_bits % 16)

See above.

> +            avpriv_request_sample(s, "Packet not ending at a 16-bit boundary");
> +
> +        return FFALIGN(pkt_size_bits, 16) >> 3;
> +    }

> +    return 0;

Please remove this line or the default branch above.

> +}
> +
> +/* Largest offset between bursts we currently handle, i.e. E-AC-3 */
> +#define SPDIF_MAX_OFFSET 24576
>
>  static int spdif_probe(const AVProbeData *p)
>  {
> @@ -140,7 +169,7 @@ int ff_spdif_probe(const uint8_t *p_buf, int buf_size, enum AVCodecID *codec)
>                  break;
>

>              /* continue probing to find more sync codes */
> -            probe_end = FFMIN(buf + SPDIF_MAX_OFFSET, p_buf + buf_size - 1);
> +            probe_end = FFMIN(buf + SPDIF_MAX_OFFSET + 1, p_buf + buf_size - 1);

Is this an unrelated fix?

>
>              /* skip directly to the next sync code */
>              if (!spdif_get_offset_and_codec(NULL, (buf[2] << 8) | buf[1],
> @@ -176,7 +205,7 @@ int ff_spdif_read_packet(AVFormatContext *s, AVPacket *pkt)
>      enum IEC61937DataType data_type;
>      enum AVCodecID codec_id;
>      uint32_t state = 0;
> -    int pkt_size_bits, offset, ret;
> +    int pkt_size, offset, ret;
>
>      while (state != (AV_BSWAP16C(SYNCWORD1) << 16 | AV_BSWAP16C(SYNCWORD2))) {
>          state = (state << 8) | avio_r8(pb);
> @@ -185,12 +214,12 @@ int ff_spdif_read_packet(AVFormatContext *s, AVPacket *pkt)
>      }
>
>      data_type = avio_rl16(pb);
> -    pkt_size_bits = avio_rl16(pb);
> +    pkt_size = spdif_read_burst_payload_length(s, data_type);
>
> -    if (pkt_size_bits % 16)
> -        avpriv_request_sample(s, "Packet not ending at a 16-bit boundary");

> +    if (!pkt_size)
> +        return AVERROR_BUG;

This looks wrong or do I miss something?

Carl Eugen
Denis Shulyaka April 11, 2021, 10:21 a.m. UTC | #2
Hi Carl,

Thanks for the review.

сб, 10 апр. 2021 г. в 18:05, Carl Eugen Hoyos <ceffmpeg@gmail.com>:

> Please use "&" instead of "%".
>

 Fixed

Please remove this line.
>

Fixed

See above.
>

Fixed

Please remove this line or the default branch above.
>

Fixed. It was there as a safety feature in case someone adds a new
condition to the switch which wouldn't return.

Is this an unrelated fix?
>

Yes, you are right, will submit as a separate patch.

> +    if (!pkt_size)
> > +        return AVERROR_BUG;
>
> This looks wrong or do I miss something?
>

This is also just for safety to verify that future modifications of
spdif_read_burst_payload_length() work correctly. Should I remove it?
diff mbox series

Patch

diff --git a/libavformat/spdifdec.c b/libavformat/spdifdec.c
index 1808fa9d65..58841e7775 100644
--- a/libavformat/spdifdec.c
+++ b/libavformat/spdifdec.c
@@ -93,6 +93,10 @@  static int spdif_get_offset_and_codec(AVFormatContext *s,
         *offset = 8192;
         *codec = AV_CODEC_ID_DTS;
         break;
+    case IEC61937_EAC3:
+        *offset = AC3_FRAME_SIZE << 4;
+        *codec = AV_CODEC_ID_EAC3;
+        break;
     default:
         if (s) { /* be silent during a probe */
             avpriv_request_sample(s, "Data type 0x%04x in IEC 61937",
@@ -103,9 +107,34 @@  static int spdif_get_offset_and_codec(AVFormatContext *s,
     return 0;
 }
 
-/* Largest offset between bursts we currently handle, i.e. AAC with
-   samples = 4096 */
-#define SPDIF_MAX_OFFSET 16384
+static int spdif_read_burst_payload_length(AVFormatContext *s,
+                                           enum IEC61937DataType data_type)
+{
+    AVIOContext *pb = s->pb;
+    int pkt_size_bits, pkt_size_bytes;
+
+    switch (data_type & 0xff) {
+    case IEC61937_EAC3:
+        pkt_size_bytes = avio_rl16(pb);
+
+        if (pkt_size_bytes % 2)
+            avpriv_request_sample(s, "Packet not ending at a 16-bit boundary");
+
+        return FFALIGN(pkt_size_bytes, 2);
+        break;
+    default:
+        pkt_size_bits = avio_rl16(pb);
+
+        if (pkt_size_bits % 16)
+            avpriv_request_sample(s, "Packet not ending at a 16-bit boundary");
+
+        return FFALIGN(pkt_size_bits, 16) >> 3;
+    }
+    return 0;
+}
+
+/* Largest offset between bursts we currently handle, i.e. E-AC-3 */
+#define SPDIF_MAX_OFFSET 24576
 
 static int spdif_probe(const AVProbeData *p)
 {
@@ -140,7 +169,7 @@  int ff_spdif_probe(const uint8_t *p_buf, int buf_size, enum AVCodecID *codec)
                 break;
 
             /* continue probing to find more sync codes */
-            probe_end = FFMIN(buf + SPDIF_MAX_OFFSET, p_buf + buf_size - 1);
+            probe_end = FFMIN(buf + SPDIF_MAX_OFFSET + 1, p_buf + buf_size - 1);
 
             /* skip directly to the next sync code */
             if (!spdif_get_offset_and_codec(NULL, (buf[2] << 8) | buf[1],
@@ -176,7 +205,7 @@  int ff_spdif_read_packet(AVFormatContext *s, AVPacket *pkt)
     enum IEC61937DataType data_type;
     enum AVCodecID codec_id;
     uint32_t state = 0;
-    int pkt_size_bits, offset, ret;
+    int pkt_size, offset, ret;
 
     while (state != (AV_BSWAP16C(SYNCWORD1) << 16 | AV_BSWAP16C(SYNCWORD2))) {
         state = (state << 8) | avio_r8(pb);
@@ -185,12 +214,12 @@  int ff_spdif_read_packet(AVFormatContext *s, AVPacket *pkt)
     }
 
     data_type = avio_rl16(pb);
-    pkt_size_bits = avio_rl16(pb);
+    pkt_size = spdif_read_burst_payload_length(s, data_type);
 
-    if (pkt_size_bits % 16)
-        avpriv_request_sample(s, "Packet not ending at a 16-bit boundary");
+    if (!pkt_size)
+        return AVERROR_BUG;
 
-    ret = av_new_packet(pkt, FFALIGN(pkt_size_bits, 16) >> 3);
+    ret = av_new_packet(pkt, pkt_size);
     if (ret)
         return ret;