[FFmpeg-devel,2/4] V11 - SCTE-35 extraction from mpegts

From: Carlos Fernandez <carlos@ccextractor.org>

Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
---
 libavformat/mpegts.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

On Tue, 27 Sep 2016, Carlos Fernandez Sanz wrote:

> From: Carlos Fernandez <carlos@ccextractor.org>
>
> Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
> ---
> libavformat/mpegts.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index b31d233..d816450 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -725,6 +725,12 @@ static const StreamType HDMV_types[] = {
>     { 0 },
> };
> 
> +/* SCTE types */
> +static const StreamType SCTE_types[] = {
> +    { 0x86, AVMEDIA_TYPE_DATA,  AV_CODEC_ID_SCTE_35    },
> +    { 0 },
> +};
> +
> /* ATSC ? */
> static const StreamType MISC_types[] = {
>     { 0x81, AVMEDIA_TYPE_AUDIO, AV_CODEC_ID_AC3 },
> @@ -872,6 +878,13 @@ static void reset_pes_packet_state(PESContext *pes)
>     av_buffer_unref(&pes->buffer);
> }
> 
> +static void new_data_packet(const uint8_t *buffer, int len, AVPacket *pkt)
> +{
> +    av_init_packet(pkt);
> +    pkt->data = buffer;
> +    pkt->size = len;
> +}
> +
> static int new_pes_packet(PESContext *pes, AVPacket *pkt)
> {
>     char *sd;
> @@ -1590,6 +1603,27 @@ static void m4sl_cb(MpegTSFilter *filter, const uint8_t *section,
>         av_free(mp4_descr[i].dec_config_descr);
> }
> 
> +static void scte_data_cb(MpegTSFilter *filter, const uint8_t *section,
> +                    int section_len)
> +{
> +    AVProgram *prg = NULL;
> +    MpegTSContext *ts = filter->u.section_filter.opaque;
> +
> +    int idx = ff_find_stream_index(ts->stream, filter->pid);

If idx can be negative (you implied it can be, I am not sure), then 
bail out as early as possible:

if (idx < 0)
     return;

> +    new_data_packet(section, section_len, ts->pkt);
> +    if (idx >= 0) {

So you don't have to check it here anymore.

> +        ts->pkt->stream_index = idx;
> +    }
> +    prg = av_find_program_from_stream(ts->stream, NULL, idx);
> +    if (prg && prg->pcr_pid != -1 && prg->discard != AVDISCARD_ALL) {
> +        MpegTSFilter *f = ts->pids[prg->pcr_pid];
> +        if (f)

if (f && f->last_pcr != -1)

> +            ts->pkt->pts = ts->pkt->dts = f->last_pcr/300;
> +    }
> +    ts->stop_parse = 1;
> +
> +}
> +
> static const uint8_t opus_coupled_stream_cnt[9] = {
>     1, 0, 1, 1, 2, 2, 2, 3, 3
> };
> @@ -1868,6 +1902,12 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
>     return 0;
> }
> 
> +static int is_pes_stream(int stream_type, uint32_t prog_reg_desc)
> +{
> +    return !(stream_type == 0x13 ||
> +             (stream_type == 0x86 && prog_reg_desc == AV_RL32("CUEI")) );

There is an extra space in the indentation.

> +}
> +
> static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len)
> {
>     MpegTSContext *ts = filter->u.section_filter.opaque;
> @@ -1975,7 +2015,8 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
>                 pes->st->id = pes->pid;
>             }
>             st = pes->st;
> -        } else if (stream_type != 0x13) {
> +
> +        } else if (is_pes_stream(stream_type, prog_reg_desc)) {
>             if (ts->pids[pid])
>                 mpegts_close_filter(ts, ts->pids[pid]); // wrongly added sdt filter probably
>             pes = add_pes_stream(ts, pid, pcr_pid);
> @@ -1995,6 +2036,10 @@ static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
>                     goto out;
>                 st->id = pid;
>                 st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
> +                if (stream_type == 0x86 && prog_reg_desc == AV_RL32("CUEI")) {
> +                    mpegts_find_stream_type(st, stream_type, SCTE_types);
> +                    mpegts_open_section_filter(ts, pid, scte_data_cb, ts, 1);
> +                }
>             }
>         }
> 
> @@ -2317,7 +2362,6 @@ static int handle_packet(MpegTSContext *ts, const uint8_t *packet)
>                 }
>             }
>         }
> -

Stray line deletion.

>     } else {
>         int ret;
>         // Note: The position here points actually behind the current packet.

Regards,
Marton

On Tue, 27 Sep 2016, Carlos Fernandez Sanz wrote:

> From: Carlos Fernandez <carlos@ccextractor.org>
>
> Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
> ---
> libavformat/mpegts.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 46 insertions(+), 2 deletions(-)
>

[...]

> +static void scte_data_cb(MpegTSFilter *filter, const uint8_t *section,
> +                    int section_len)
> +{
> +    AVProgram *prg = NULL;
> +    MpegTSContext *ts = filter->u.section_filter.opaque;
> +
> +    int idx = ff_find_stream_index(ts->stream, filter->pid);
> +    new_data_packet(section, section_len, ts->pkt);

One more thing you might consider: now, you are creating a data packet 
from the whole section, including the header and the rest. Since the 
section is already parsed, and the crc is already checked, and as far 
as I see there is nothing useful in the header, it might make 
sense to create a data packet only from the buffer _after_ the 
section_length field, skipping the last CRC32 field as well.

This way, the SCTE codec does not have to do any section parsing at all, 
which seems a cleaner solution to me. What do you think?

Regards,
Marton

On Tue, Sep 27, 2016 at 3:13 PM, Marton Balint <cus@passwd.hu> wrote:
>
> nothing useful in the header, it might make sense to create a data packet
> only from the buffer _after_ the section_length field, skipping the last
> CRC32 field as well.

I've been looking into this, but it would break some things (like
base64 encoding of complete scte-35 packet) . Plus we'd miss the
table_id which is not parsed in mpets.c.

Next version ready (will submit now) with other things that have been
mentioned here.