diff mbox series

[FFmpeg-devel,2/2] avformat/mpegts: parse for AC-3 descriptor 6A

Message ID 1597415717-22817-2-git-send-email-lance.lmwang@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,1/2] avformat/mpegtsenc: support DVB 6A descriptor for AC-3 | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang Aug. 14, 2020, 2:35 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/mpegts.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 6 deletions(-)

Comments

Marton Balint Aug. 14, 2020, 8:08 p.m. UTC | #1 
On Fri, 14 Aug 2020, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>

I don't think this patch fits into libavformat. I am not a fan of dumping 
the descriptor data with AV_LOG_DEBUG, because libavformat is not a 
stream analyzer. Also I don't see much benefit of parsing the whole 
descriptor, when you are not doing anything with the parsed value.

Sorry, but I think we are better off if we not apply this.

Regards,
Marton


>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavformat/mpegts.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index f71f18a5..72cc72a 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -2075,16 +2075,57 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
>         break;
>     case 0x6a: /* ac-3_descriptor */
>         {
> -            int component_type_flag = get8(pp, desc_end) & (1 << 7);
> -            if (component_type_flag) {
> -                int component_type = get8(pp, desc_end);
> -                int service_type_mask = 0x38;  // 0b00111000
> -                int service_type = ((component_type & service_type_mask) >> 3);
> +            AVDVBAC3Descriptor desc6a;
> +            uint8_t buf;
> +
> +            if (desc_end - *pp < 1)
> +                return AVERROR_INVALIDDATA;
> +
> +            buf = get8(pp, desc_end);
> +            desc6a.component_type_flag = (buf >> 7) & 0x1;
> +            desc6a.bsid_flag           = (buf >> 6) & 0x1;
> +            desc6a.mainid_flag         = (buf >> 5) & 0x1;
> +            desc6a.asvc_flag           = (buf >> 4) & 0x1;
> +
> +            av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "Stream[0x%x] AC-3(6a):", st->id);
> +            if (desc6a.component_type_flag) {
> +                int service_type;
> +                int number_of_channels;
> +                desc6a.component_type = get8(pp, desc_end);
> +                service_type = ((desc6a.component_type >> 3) & 0x7);
>                 if (service_type == 0x02 /* 0b010 */) {
>                     st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
> -                    av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "New track disposition for id %u: %u\n", st->id, st->disposition);
> +                    av_log(ts ? ts->stream : fc, AV_LOG_DEBUG,
> +                           " disposition: 0x%x", st->disposition);
> +                }
> +                number_of_channels = desc6a.component_type & 0x7;
> +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " component_type: 0x%x number_of_channels: %u",
> +                        desc6a.component_type, number_of_channels);
> +            }
> +            if (desc6a.bsid_flag) {
> +                if (desc_end - *pp < 1) {
> +                    return AVERROR_INVALIDDATA;
> +                }
> +                desc6a.bsid = get8(pp, desc_end);
> +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " bsid: %u", desc6a.bsid);
> +            }
> +            if (desc6a.mainid_flag) {
> +                if (desc_end - *pp < 1) {
> +                    return AVERROR_INVALIDDATA;
> +                }
> +                desc6a.mainid = get8(pp, desc_end);
> +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " mainid: %u", desc6a.mainid);
> +            }
> +            if (desc6a.asvc_flag) {
> +                if (desc_end - *pp < 1) {
> +                    return AVERROR_INVALIDDATA;
>                 }
> +                desc6a.asvc = get8(pp, desc_end);
> +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " asvc: %u", desc6a.asvc);
>             }
> +            if (desc6a.component_type_flag || desc6a.bsid_flag ||
> +                desc6a.mainid_flag         || desc6a.asvc_flag)
> +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "\n");
>         }
>         break;
>     case 0x7a: /* enhanced_ac-3_descriptor */
> -- 
> 1.8.3.1
>
> _______________________________________________
> 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".
Lance Wang Aug. 15, 2020, 12:56 a.m. UTC | #2 
On Fri, Aug 14, 2020 at 10:08:06PM +0200, Marton Balint wrote:
> 
> 
> On Fri, 14 Aug 2020, lance.lmwang@gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> 
> I don't think this patch fits into libavformat. I am not a fan of dumping
> the descriptor data with AV_LOG_DEBUG, because libavformat is not a stream
> analyzer. Also I don't see much benefit of parsing the whole descriptor,
> when you are not doing anything with the parsed value.
> 
> Sorry, but I think we are better off if we not apply this.

We can get the audio channels and channel_layout by the descriptor
from demuxer, it's always used by IRD. But I'm not sure whether it's 
make sense to add them for no sample rate and bitrate here. So now I
use it help to check next patch result without using stream analyzer.

> 
> Regards,
> Marton
> 
> 
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > libavformat/mpegts.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 47 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index f71f18a5..72cc72a 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -2075,16 +2075,57 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
> >         break;
> >     case 0x6a: /* ac-3_descriptor */
> >         {
> > -            int component_type_flag = get8(pp, desc_end) & (1 << 7);
> > -            if (component_type_flag) {
> > -                int component_type = get8(pp, desc_end);
> > -                int service_type_mask = 0x38;  // 0b00111000
> > -                int service_type = ((component_type & service_type_mask) >> 3);
> > +            AVDVBAC3Descriptor desc6a;
> > +            uint8_t buf;
> > +
> > +            if (desc_end - *pp < 1)
> > +                return AVERROR_INVALIDDATA;
> > +
> > +            buf = get8(pp, desc_end);
> > +            desc6a.component_type_flag = (buf >> 7) & 0x1;
> > +            desc6a.bsid_flag           = (buf >> 6) & 0x1;
> > +            desc6a.mainid_flag         = (buf >> 5) & 0x1;
> > +            desc6a.asvc_flag           = (buf >> 4) & 0x1;
> > +
> > +            av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "Stream[0x%x] AC-3(6a):", st->id);
> > +            if (desc6a.component_type_flag) {
> > +                int service_type;
> > +                int number_of_channels;
> > +                desc6a.component_type = get8(pp, desc_end);
> > +                service_type = ((desc6a.component_type >> 3) & 0x7);
> >                 if (service_type == 0x02 /* 0b010 */) {
> >                     st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
> > -                    av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "New track disposition for id %u: %u\n", st->id, st->disposition);
> > +                    av_log(ts ? ts->stream : fc, AV_LOG_DEBUG,
> > +                           " disposition: 0x%x", st->disposition);
> > +                }
> > +                number_of_channels = desc6a.component_type & 0x7;
> > +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " component_type: 0x%x number_of_channels: %u",
> > +                        desc6a.component_type, number_of_channels);
> > +            }
> > +            if (desc6a.bsid_flag) {
> > +                if (desc_end - *pp < 1) {
> > +                    return AVERROR_INVALIDDATA;
> > +                }
> > +                desc6a.bsid = get8(pp, desc_end);
> > +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " bsid: %u", desc6a.bsid);
> > +            }
> > +            if (desc6a.mainid_flag) {
> > +                if (desc_end - *pp < 1) {
> > +                    return AVERROR_INVALIDDATA;
> > +                }
> > +                desc6a.mainid = get8(pp, desc_end);
> > +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " mainid: %u", desc6a.mainid);
> > +            }
> > +            if (desc6a.asvc_flag) {
> > +                if (desc_end - *pp < 1) {
> > +                    return AVERROR_INVALIDDATA;
> >                 }
> > +                desc6a.asvc = get8(pp, desc_end);
> > +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " asvc: %u", desc6a.asvc);
> >             }
> > +            if (desc6a.component_type_flag || desc6a.bsid_flag ||
> > +                desc6a.mainid_flag         || desc6a.asvc_flag)
> > +                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "\n");
> >         }
> >         break;
> >     case 0x7a: /* enhanced_ac-3_descriptor */
> > -- 
> > 1.8.3.1
> > 
> > _______________________________________________
> > 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".
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index f71f18a5..72cc72a 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -2075,16 +2075,57 @@  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
         break;
     case 0x6a: /* ac-3_descriptor */
         {
-            int component_type_flag = get8(pp, desc_end) & (1 << 7);
-            if (component_type_flag) {
-                int component_type = get8(pp, desc_end);
-                int service_type_mask = 0x38;  // 0b00111000
-                int service_type = ((component_type & service_type_mask) >> 3);
+            AVDVBAC3Descriptor desc6a;
+            uint8_t buf;
+
+            if (desc_end - *pp < 1)
+                return AVERROR_INVALIDDATA;
+
+            buf = get8(pp, desc_end);
+            desc6a.component_type_flag = (buf >> 7) & 0x1;
+            desc6a.bsid_flag           = (buf >> 6) & 0x1;
+            desc6a.mainid_flag         = (buf >> 5) & 0x1;
+            desc6a.asvc_flag           = (buf >> 4) & 0x1;
+
+            av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "Stream[0x%x] AC-3(6a):", st->id);
+            if (desc6a.component_type_flag) {
+                int service_type;
+                int number_of_channels;
+                desc6a.component_type = get8(pp, desc_end);
+                service_type = ((desc6a.component_type >> 3) & 0x7);
                 if (service_type == 0x02 /* 0b010 */) {
                     st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
-                    av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "New track disposition for id %u: %u\n", st->id, st->disposition);
+                    av_log(ts ? ts->stream : fc, AV_LOG_DEBUG,
+                           " disposition: 0x%x", st->disposition);
+                }
+                number_of_channels = desc6a.component_type & 0x7;
+                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " component_type: 0x%x number_of_channels: %u",
+                        desc6a.component_type, number_of_channels);
+            }
+            if (desc6a.bsid_flag) {
+                if (desc_end - *pp < 1) {
+                    return AVERROR_INVALIDDATA;
+                }
+                desc6a.bsid = get8(pp, desc_end);
+                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " bsid: %u", desc6a.bsid);
+            }
+            if (desc6a.mainid_flag) {
+                if (desc_end - *pp < 1) {
+                    return AVERROR_INVALIDDATA;
+                }
+                desc6a.mainid = get8(pp, desc_end);
+                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " mainid: %u", desc6a.mainid);
+            }
+            if (desc6a.asvc_flag) {
+                if (desc_end - *pp < 1) {
+                    return AVERROR_INVALIDDATA;
                 }
+                desc6a.asvc = get8(pp, desc_end);
+                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, " asvc: %u", desc6a.asvc);
             }
+            if (desc6a.component_type_flag || desc6a.bsid_flag ||
+                desc6a.mainid_flag         || desc6a.asvc_flag)
+                av_log(ts ? ts->stream : fc, AV_LOG_DEBUG, "\n");
         }
         break;
     case 0x7a: /* enhanced_ac-3_descriptor */