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

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(-)

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) {

> +            // This structure is defined in STD-B10, part 1, listing 5.4 and
> +            // part 2, 6.2.20).
> +            // Listing of data_component_ids is in STD-B10, part 2, Annex J.
> +            // Component tag limits are documented in TR-B14, fascicle 2,
> +            // Vol. 3, Section 2, 4.2.8.1
> +            int actual_component_tag = st->stream_identifier - 1;
> +            int picked_profile = FF_PROFILE_UNKNOWN;
> +            int data_component_id = get16(pp, desc_end);
> +            if (data_component_id < 0)
> +                return AVERROR_INVALIDDATA;
> +
> +            switch (data_component_id) {
> +            case 0x0008:
> +                // [0x30..0x37] are component tags utilized for
> +                // non-mobile captioning service ("profile A").
> +                if (actual_component_tag >= 0x30 &&
> +                    actual_component_tag <= 0x37) {
> +                    picked_profile = FF_PROFILE_ARIB_PROFILE_A;
> +                }
> +                break;
> +            case 0x0012:
> +                // component tag 0x87 signifies a mobile/partial reception
> +                // (1seg) captioning service ("profile C").
> +                if (actual_component_tag == 0x87) {
> +                    picked_profile = FF_PROFILE_ARIB_PROFILE_C;
> +                }
> +                break;
> +            default:
> +                break;
> +            }
> +
> +            if (picked_profile == FF_PROFILE_UNKNOWN)
> +                break;
> +
> +            st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
> +            st->codecpar->codec_id   = AV_CODEC_ID_ARIB_CAPTION;
> +            st->codecpar->profile    = picked_profile;
> +            st->request_probe        = 0;
> +        }
> +        break;
>     default:
>         break;
>     }
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 4408ecaa06..2e83eb4f23 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -33,7 +33,7 @@
> // Also please add any ticket numbers that you believe might be affected here
> #define LIBAVFORMAT_VERSION_MAJOR  58
> #define LIBAVFORMAT_VERSION_MINOR  26
> -#define LIBAVFORMAT_VERSION_MICRO 100
> +#define LIBAVFORMAT_VERSION_MICRO 101
> 
> #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
>                                                LIBAVFORMAT_VERSION_MINOR, \
> --

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

Thanks,
Marton

On Sat, Feb 2, 2019 at 6:13 PM Marton Balint <cus@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