[FFmpeg-devel] [PATCH 2/2] lavf/mpegts: add reading of ARIB data coding descriptor

[Jan Ekström]

On Sat, Feb 2, 2019 at 6:13 PM Marton Balint <cus at passwd.hu> wrote:
>
>
>
> On Sat, 2 Feb 2019, Jan Ekström wrote:
>
> > This enables us to read the data coding type utilized for
> > a specific private data stream, of which we currently are
> > interested in ARIB caption streams.
> >
> > The component tag limitations are according to ARIB TR-B14,
> > and the component IDs are defined in ARIB STD-B10.
> > ---
> > libavformat/mpegts.c  | 47 +++++++++++++++++++++++++++++++++++++++++++
> > libavformat/version.h |  2 +-
> > 2 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index 300db110d4..a170b3e1b6 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -2013,6 +2013,53 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
> >             }
> >         }
> >         break;
> > +    case 0xfd: /* ARIB data coding type descriptor */
> > +        if (stream_type != STREAM_TYPE_PRIVATE_DATA)
> > +            // STD-B24, fascicle 3, chapter 4 defines private_stream_1
> > +            // for captions
> > +            break;
> > +
> > +        {
>
> If you are putting code in the block anyway, then why don't you simply
> write:
>
> if (stream_type == STREAM_TYPE_PRIVATE_DATA) {
>

My first idea was to follow the principle of early exit, but you do
indeed lose the benefits of no extra indentation with putting the
longer piece of code into a block. I don't have a hard preference on
this.

>
> Otherwise LGTM, although I can't really comment on the ARIB stuff.
>

Right. Something I forgot to mention this in the poster mail.

The English versions of the specifications are still available for free at:
https://www.arib.or.jp/english/std_tr/broadcasting/sb_ej.html (STD-BXX
standards)
https://www.arib.or.jp/english/std_tr/broadcasting/rb_ej.html (TR-BXX
recommendations)
(some are out of date but I don't think these things have largely
changed after 2006-7 or so after single-segment mobile broadcasting
was started - so for the purposes of this review it should be OK).

I think the worst part is where they split the specification into X
parts so you have to make sure you've marked your references
correctly.

Jan