[FFmpeg-devel] avformat/mpegts: set AV_DISPOSITION_DESCRIPTIONS for OIPF cases

Submitted by Łukasz Krzciuk on April 16, 2018, 7:16 a.m.

Details

Message ID CAFCngT=3cXQje18zLm=ffN_vkKnZF4-PukeaiJ8=Z78mZ9rhUg@mail.gmail.com
State New
Headers show

Commit Message

Łukasz Krzciuk April 16, 2018, 7:16 a.m.
I agree, AV_LOG_INFO has been changed to AV_LOG_DEBUG.

Regards,

*Łukasz Krzciuk*
Developer

Vewd
ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska


On Fri, Apr 13, 2018 at 5:50 PM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Fri, Apr 13, 2018 at 03:44:40PM +0200, Łukasz Krzciuk wrote:
> > Hi,
> >
> > this change is needed according to OIPF spec: 8.4.2 AVComponent,
> > audioDescription case:
> >
> >    1. an audio component with an ISO_639_language_descriptor in the PMT
> >    with the audio_type field set to 0x03
> >    2. a supplementary_audio_descriptor with the editorial_classification
> >    field set to 0x01
> >    3. an ac-3_descriptor or an enhanced_ac-3_descriptor with a
> >    component_type field with the service_type flags set to Visually
> Impaired
> >
> >
> > Regards,
> >
> > *Łukasz Krzciuk*
> > Developer
> >
> > Vewd
> > ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska
>
> >  mpegts.c |   30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 863b7bca7f23f9ea337fde33f5bb9265ef3b4270  0001-avformat-mpegts-set-AV_
> DISPOSITION_DESCRIPTIONS-for-.patch
> > From 12ef5cdd9c443b1601dc98d910feac87233b1040 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?=C5=81ukasz=20Krzciuk?= <lkrzciuk@vewd.com>
> > Date: Fri, 13 Apr 2018 14:57:57 +0200
> > Subject: [PATCH] avformat/mpegts: set AV_DISPOSITION_DESCRIPTIONS for
> OIPF
> >  cases
> >
> > 1. an audio component with an ISO_639_language_descriptor in the PMT
> with the
> > audio_type field set to 0x03
> > 2. a supplementary_audio_descriptor with the editorial_classification
> field set
> > to 0x01
> > 3. an ac-3_descriptor or an enhanced_ac-3_descriptor with a
> component_type field
> > with the service_type flags set to Visually Impaired
> > ---
> >  libavformat/mpegts.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index 37a6aa8..df2b3aa 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -1835,6 +1835,7 @@ int ff_parse_mpeg2_descriptor(AVFormatContext
> *fc, AVStream *st, int stream_type
> >                  break;
> >              case 0x03:
> >                  st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
> > +                st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
> >                  break;
> >              }
> >          }
> > @@ -1910,6 +1911,7 @@ int ff_parse_mpeg2_descriptor(AVFormatContext
> *fc, AVStream *st, int stream_type
> >              switch ((flags >> 2) & 0x1F) { /* editorial_classification
> */
> >              case 0x01:
> >                  st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
> > +                st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
> >                  break;
> >              case 0x02:
> >                  st->disposition |= AV_DISPOSITION_HEARING_IMPAIRED;
> > @@ -1934,6 +1936,34 @@ 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);
> > +                if (service_type == 0x02 /* 0b010 */) {
> > +                    st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
> > +                    av_log(ts->stream, AV_LOG_INFO, "New track
> disposition for id %u: %u\n", st->id, st->disposition);
>
> AV_LOG_INFO is likely too noisy for normal usecases
>
> thx
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Rewriting code that is poorly written but fully understood is good.
> Rewriting code that one doesnt understand is a sign that one is less smart
> then the original author, trying to rewrite it will not make it better.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>

Comments

Łukasz Krzciuk April 18, 2018, 6:28 a.m.
Any updates on this?

Regards,

*Łukasz Krzciuk*
Developer

Vewd
ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska


On Mon, Apr 16, 2018 at 9:16 AM, Łukasz Krzciuk <lkrzciuk@vewd.com> wrote:

> I agree, AV_LOG_INFO has been changed to AV_LOG_DEBUG.
>
> Regards,
>
> *Łukasz Krzciuk*
> Developer
>
> Vewd
> ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska
>
>
> On Fri, Apr 13, 2018 at 5:50 PM, Michael Niedermayer <
> michael@niedermayer.cc> wrote:
>
>> On Fri, Apr 13, 2018 at 03:44:40PM +0200, Łukasz Krzciuk wrote:
>> > Hi,
>> >
>> > this change is needed according to OIPF spec: 8.4.2 AVComponent,
>> > audioDescription case:
>> >
>> >    1. an audio component with an ISO_639_language_descriptor in the PMT
>> >    with the audio_type field set to 0x03
>> >    2. a supplementary_audio_descriptor with the editorial_classification
>> >    field set to 0x01
>> >    3. an ac-3_descriptor or an enhanced_ac-3_descriptor with a
>> >    component_type field with the service_type flags set to Visually
>> Impaired
>> >
>> >
>> > Regards,
>> >
>> > *Łukasz Krzciuk*
>> > Developer
>> >
>> > Vewd
>> > ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska
>>
>> >  mpegts.c |   30 ++++++++++++++++++++++++++++++
>> >  1 file changed, 30 insertions(+)
>> > 863b7bca7f23f9ea337fde33f5bb9265ef3b4270
>> 0001-avformat-mpegts-set-AV_DISPOSITION_DESCRIPTIONS-for-.patch
>> > From 12ef5cdd9c443b1601dc98d910feac87233b1040 Mon Sep 17 00:00:00 2001
>> > From: =?UTF-8?q?=C5=81ukasz=20Krzciuk?= <lkrzciuk@vewd.com>
>> > Date: Fri, 13 Apr 2018 14:57:57 +0200
>> > Subject: [PATCH] avformat/mpegts: set AV_DISPOSITION_DESCRIPTIONS for
>> OIPF
>> >  cases
>> >
>> > 1. an audio component with an ISO_639_language_descriptor in the PMT
>> with the
>> > audio_type field set to 0x03
>> > 2. a supplementary_audio_descriptor with the editorial_classification
>> field set
>> > to 0x01
>> > 3. an ac-3_descriptor or an enhanced_ac-3_descriptor with a
>> component_type field
>> > with the service_type flags set to Visually Impaired
>> > ---
>> >  libavformat/mpegts.c | 30 ++++++++++++++++++++++++++++++
>> >  1 file changed, 30 insertions(+)
>> >
>> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>> > index 37a6aa8..df2b3aa 100644
>> > --- a/libavformat/mpegts.c
>> > +++ b/libavformat/mpegts.c
>> > @@ -1835,6 +1835,7 @@ int ff_parse_mpeg2_descriptor(AVFormatContext
>> *fc, AVStream *st, int stream_type
>> >                  break;
>> >              case 0x03:
>> >                  st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
>> > +                st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
>> >                  break;
>> >              }
>> >          }
>> > @@ -1910,6 +1911,7 @@ int ff_parse_mpeg2_descriptor(AVFormatContext
>> *fc, AVStream *st, int stream_type
>> >              switch ((flags >> 2) & 0x1F) { /* editorial_classification
>> */
>> >              case 0x01:
>> >                  st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
>> > +                st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
>> >                  break;
>> >              case 0x02:
>> >                  st->disposition |= AV_DISPOSITION_HEARING_IMPAIRED;
>> > @@ -1934,6 +1936,34 @@ 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);
>> > +                if (service_type == 0x02 /* 0b010 */) {
>> > +                    st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
>> > +                    av_log(ts->stream, AV_LOG_INFO, "New track
>> disposition for id %u: %u\n", st->id, st->disposition);
>>
>> AV_LOG_INFO is likely too noisy for normal usecases
>>
>> thx
>>
>> [...]
>>
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Rewriting code that is poorly written but fully understood is good.
>> Rewriting code that one doesnt understand is a sign that one is less smart
>> then the original author, trying to rewrite it will not make it better.
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
>
Michael Niedermayer April 18, 2018, 9:03 p.m.
On Wed, Apr 18, 2018 at 08:28:05AM +0200, Łukasz Krzciuk wrote:
> Any updates on this?

has someone crosschecked this with the spec ?

[...]
Łukasz Krzciuk April 24, 2018, 8:36 a.m.
Any updates on this?

Regards,

*Łukasz Krzciuk*
Developer

Vewd
ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska


On Wed, Apr 18, 2018 at 11:03 PM, Michael Niedermayer <
michael@niedermayer.cc> wrote:

> On Wed, Apr 18, 2018 at 08:28:05AM +0200, Łukasz Krzciuk wrote:
> > Any updates on this?
>
> has someone crosschecked this with the spec ?
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many that live deserve death. And some that die deserve life. Can you give
> it to them? Then do not be too eager to deal out death in judgement. For
> even the very wise cannot see all ends. -- Gandalf
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Michael Niedermayer April 25, 2018, 12:09 a.m.
On Tue, Apr 24, 2018 at 10:36:50AM +0200, Łukasz Krzciuk wrote:
> Any updates on this?

did you base this on teh specification / have checked that it matches
the specification ?

[...]
Łukasz Krzciuk April 25, 2018, 6:17 a.m.
Yes, I have checked it and I implemented it according to OIPF spec: 8.4.2
AVComponent, audioDescription case (as I wrote in my 1st email). This
implementation is tested by official org.hbbtv_HTML50420 testcase.

Regards,

*Łukasz Krzciuk*
Developer

Vewd
ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska


On Wed, Apr 25, 2018 at 2:09 AM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Tue, Apr 24, 2018 at 10:36:50AM +0200, Łukasz Krzciuk wrote:
> > Any updates on this?
>
> did you base this on teh specification / have checked that it matches
> the specification ?
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many that live deserve death. And some that die deserve life. Can you give
> it to them? Then do not be too eager to deal out death in judgement. For
> even the very wise cannot see all ends. -- Gandalf
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Michael Niedermayer April 26, 2018, 12:05 a.m.
On Wed, Apr 25, 2018 at 08:17:38AM +0200, Łukasz Krzciuk wrote:
> Yes, I have checked it and I implemented it according to OIPF spec: 8.4.2
> AVComponent, audioDescription case (as I wrote in my 1st email). This
> implementation is tested by official org.hbbtv_HTML50420 testcase.

ok, ill apply it then

is it (easily) possibly to add some test for this to fate ?

[...]
Łukasz Krzciuk April 26, 2018, 7:22 a.m.
Thank you.

This change is really simple: set AV_DISPOSITION_DESCRIPTIONS bit when we
have a specified stream. So a test could be simple too. But I cannot see
many of tests in .../libavformat/tests dir. It seems 'disposition' is not
tested at all currently. So if I will prepare a testcase for
'ff_parse_mpeg2_descriptor' function, then a test data could be a problem
here. org.hbbtv_HTML50420 test is rather huge, and cannot be simply applied
here.


Regards,

*Łukasz Krzciuk*
Developer

Vewd
ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska


On Thu, Apr 26, 2018 at 2:05 AM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Wed, Apr 25, 2018 at 08:17:38AM +0200, Łukasz Krzciuk wrote:
> > Yes, I have checked it and I implemented it according to OIPF spec: 8.4.2
> > AVComponent, audioDescription case (as I wrote in my 1st email). This
> > implementation is tested by official org.hbbtv_HTML50420 testcase.
>
> ok, ill apply it then
>
> is it (easily) possibly to add some test for this to fate ?
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> it is not once nor twice but times without number that the same ideas make
> their appearance in the world. -- Aristotle
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Łukasz Krzciuk May 9, 2018, 8:19 a.m.
Any updates on this issue?

Regards,

*Łukasz Krzciuk*
Developer

Vewd
ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska


On Thu, Apr 26, 2018 at 9:22 AM, Łukasz Krzciuk <lkrzciuk@vewd.com> wrote:

> Thank you.
>
> This change is really simple: set AV_DISPOSITION_DESCRIPTIONS bit when we
> have a specified stream. So a test could be simple too. But I cannot see
> many of tests in .../libavformat/tests dir. It seems 'disposition' is not
> tested at all currently. So if I will prepare a testcase for
> 'ff_parse_mpeg2_descriptor' function, then a test data could be a problem
> here. org.hbbtv_HTML50420 test is rather huge, and cannot be simply applied
> here.
>
>
> Regards,
>
> *Łukasz Krzciuk*
> Developer
>
> Vewd
> ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska
>
>
> On Thu, Apr 26, 2018 at 2:05 AM, Michael Niedermayer <
> michael@niedermayer.cc> wrote:
>
>> On Wed, Apr 25, 2018 at 08:17:38AM +0200, Łukasz Krzciuk wrote:
>> > Yes, I have checked it and I implemented it according to OIPF spec:
>> 8.4.2
>> > AVComponent, audioDescription case (as I wrote in my 1st email). This
>> > implementation is tested by official org.hbbtv_HTML50420 testcase.
>>
>> ok, ill apply it then
>>
>> is it (easily) possibly to add some test for this to fate ?
>>
>> [...]
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> it is not once nor twice but times without number that the same ideas make
>> their appearance in the world. -- Aristotle
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
>
Michael Niedermayer May 10, 2018, 1:09 a.m.
On Wed, May 09, 2018 at 10:19:26AM +0200, Łukasz Krzciuk wrote:
> Any updates on this issue?

about adding a testcase ?
adding a huge sample file is not possible.
It either has to be made smaller or generated by a muxer
or last resort, no test

[...]
Łukasz Krzciuk May 10, 2018, 8:39 a.m.
Lets skip a testcase. It will be not easy to me to prepare it.

Regards,

*Łukasz Krzciuk*
Developer

Vewd
ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska


On Thu, May 10, 2018 at 3:09 AM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Wed, May 09, 2018 at 10:19:26AM +0200, Łukasz Krzciuk wrote:
> > Any updates on this issue?
>
> about adding a testcase ?
> adding a huge sample file is not possible.
> It either has to be made smaller or generated by a muxer
> or last resort, no test
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The worst form of inequality is to try to make unequal things equal.
> -- Aristotle
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Aman Gupta May 10, 2018, 6:54 p.m.
On Thu, May 10, 2018 at 1:46 AM Łukasz Krzciuk <lkrzciuk@vewd.com> wrote:

> Lets skip a testcase. It will be not easy to me to prepare it.


It should be simple to extract a small sample containing the PMT. If you
link some larger samples I can help extract the relevant bytes for test
case.

Aman


>
> Regards,
>
> *Łukasz Krzciuk*
> Developer
>
> Vewd
> ul. Grabarska 2
> <https://maps.google.com/?q=Grabarska+2&entry=gmail&source=g>, Pegaz 2A,
> 50-079 Wrocław, Polska
>
>
> On Thu, May 10, 2018 at 3:09 AM, Michael Niedermayer
> <michael@niedermayer.cc
> > wrote:
>
> > On Wed, May 09, 2018 at 10:19:26AM +0200, Łukasz Krzciuk wrote:
> > > Any updates on this issue?
> >
> > about adding a testcase ?
> > adding a huge sample file is not possible.
> > It either has to be made smaller or generated by a muxer
> > or last resort, no test
> >
> > [...]
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > The worst form of inequality is to try to make unequal things equal.
> > -- Aristotle
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Patch hide | download patch | download mbox

From fc2e281eb0e0e526fee52944205d6bb3729a17f0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Krzciuk?= <lkrzciuk@vewd.com>
Date: Fri, 13 Apr 2018 14:57:57 +0200
Subject: [PATCH] avformat/mpegts: set AV_DISPOSITION_DESCRIPTIONS for OIPF
 cases

1. an audio component with an ISO_639_language_descriptor in the PMT with the
audio_type field set to 0x03
2. a supplementary_audio_descriptor with the editorial_classification field set
to 0x01
3. an ac-3_descriptor or an enhanced_ac-3_descriptor with a component_type field
with the service_type flags set to Visually Impaired
---
 libavformat/mpegts.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 37a6aa8..629631f 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -1835,6 +1835,7 @@  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
                 break;
             case 0x03:
                 st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
+                st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
                 break;
             }
         }
@@ -1910,6 +1911,7 @@  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
             switch ((flags >> 2) & 0x1F) { /* editorial_classification */
             case 0x01:
                 st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
+                st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
                 break;
             case 0x02:
                 st->disposition |= AV_DISPOSITION_HEARING_IMPAIRED;
@@ -1934,6 +1936,34 @@  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);
+                if (service_type == 0x02 /* 0b010 */) {
+                    st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
+                    av_log(ts->stream, AV_LOG_DEBUG, "New track disposition for id %u: %u\n", st->id, st->disposition);
+                }
+            }
+        }
+        break;
+    case 0x7a: /* enhanced_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);
+                if (service_type == 0x02 /* 0b010 */) {
+                    st->disposition |= AV_DISPOSITION_DESCRIPTIONS;
+                    av_log(ts->stream, AV_LOG_DEBUG, "New track disposition for id %u: %u\n", st->id, st->disposition);
+                }
+            }
+        }
+        break;
     default:
         break;
     }
-- 
2.8.3