[FFmpeg-devel] DVB EPG decoder

Hi

I'm still interested to have those three commits merged (update in attachments).

But I'd like to see data decoder in the future to use more easily
EPG/NIT/BAT etc tables.
Will it be possible? With modifications if it needs to be?

Anthony Delannoy

Le mar. 3 sept. 2019 à 23:39, Marton Balint <cus@passwd.hu> a écrit :
>
> Hi,
>
> I am not sure if you are interested getting only patch 1, 2 and 3
> merged, but if you are, then here are my comments for patch 3.
>
> > From 025ec8e8d607d02f2e5b4021783ab8f3b42d0bc1 Mon Sep 17 00:00:00 2001
> > From: Anthony Delannoy <anthony.2lannoy@gmail.com>
> > Date: Wed, 21 Aug 2019 11:46:56 +0200
> > Subject: [PATCH 03/10] lavf/mpegts: EPG extraction from mpegts
> >
> > ---
> >  libavformat/mpegts.c | 67 ++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 65 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index 47d8d5f877..03c1753ac7 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -2489,13 +2489,57 @@ static void pat_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
> >      }
> >  }
> >
> > +static void eit_cb(MpegTSFilter *filter, const uint8_t *section, int section_len)
> > +{
> > +    MpegTSContext *ts = filter->u.section_filter.opaque;
> > +    MpegTSSectionFilter *tssf = &filter->u.section_filter;
> > +    const uint8_t *p, *p_end;
> > +    SectionHeader h1, *h = &h1;
> > +    int idx;
> > +    AVProgram *prg;
>
> Maybe you can check if AVStream->discard == DISCARD_ALL for the EIT stream and
> skip the packet if the stream is discarded.
>
> > +
> > +    p_end = section + section_len - 4;
> > +    p     = section;
> > +
> > +    if (parse_section_header(h, &p, p_end) < 0)
> > +        return;
> > +    if (h->tid < EIT_TID || h->tid > OEITS_END_TID)
> > +        return;
>
> > +    if (ts->skip_changes)
> > +        return;
> > +    if (skip_identical(h, tssf))
> > +        return;
>
> I don't think these last two checks (skip changes and skip identical) makes
> much sense here, just remove them.
>
> > +
> > +    idx = ff_find_stream_index(ts->stream, filter->pid);
> > +    if (idx < 0)
> > +        return;
>
> Instead of finding the stream each time, you should simply add an AVStream
> *epg_stream to MpegTsContext, set it where you create the stream, and use that
> always.
>
> > +
> > +    /**
> > +     * In case we receive an EPG packet before mpegts context is fully
> > +     * initialized.
> > +     */
> > +    if (!ts->pkt)
> > +        return;
> > +
> > +    new_data_packet(section, section_len, ts->pkt);
> > +    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 && f->last_pcr != -1)
> > +            ts->pkt->pts = ts->pkt->dts = f->last_pcr/300;
> > +    }
>
> This program based logic is no longer needed since EIT is not added to programs.
>
> > +    ts->stop_parse = 1;
> > +}
> > +
> >  static void sdt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len)
> >  {
> >      MpegTSContext *ts = filter->u.section_filter.opaque;
> >      MpegTSSectionFilter *tssf = &filter->u.section_filter;
> >      SectionHeader h1, *h = &h1;
> >      const uint8_t *p, *p_end, *desc_list_end, *desc_end;
> > -    int onid, val, sid, desc_list_len, desc_tag, desc_len, service_type;
> > +    int onid, val, sid, desc_list_len, desc_tag, desc_len, service_type,
> > +        eit_sched, eit_pres_following;
> >      char *name, *provider_name;
> >
> >      av_log(ts->stream, AV_LOG_TRACE, "SDT:\n");
> > @@ -2525,6 +2569,24 @@ static void sdt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
> >          val = get8(&p, p_end);
> >          if (val < 0)
> >              break;
> > +        eit_sched = (val >> 1) & 0x1;
> > +        eit_pres_following = val & 0x1;
> > +
> > +        if (eit_sched | eit_pres_following) {
> > +            int idx = ff_find_stream_index(ts->stream, EIT_PID);
> > +            AVStream *st = (idx >= 0) ? ts->stream->streams[EIT_PID] : NULL;
>
> I guess ts->stream->streams[EIT_PID] wanted to be ts->stream->streams[idx] but
> it does not matter because you should rework this if you add epg_stream to
> MpegTsContext.
>
> > +
> > +            if (!st) {
> > +                st = avformat_new_stream(ts->stream, NULL);
> > +                if (!st)
> > +                    return;
> > +                st->id = EIT_PID;
> > +                st->program_num = sid;
> > +                st->codecpar->codec_type = AVMEDIA_TYPE_DATA;
> > +                st->codecpar->codec_id = AV_CODEC_ID_EPG;
> > +            }
> > +        }
> > +
> >          desc_list_len = get16(&p, p_end);
> >          if (desc_list_len < 0)
> >              break;
> > @@ -2975,8 +3037,8 @@ static int mpegts_read_header(AVFormatContext *s)
> >          seek_back(s, pb, pos);
> >
> >          mpegts_open_section_filter(ts, SDT_PID, sdt_cb, ts, 1);
> > -
> >          mpegts_open_section_filter(ts, PAT_PID, pat_cb, ts, 1);
> > +        mpegts_open_section_filter(ts, EIT_PID, eit_cb, ts, 1);
> >
> >          handle_packets(ts, probesize / ts->raw_packet_size);
> >          /* if could not find service, enable auto_guess */
> > @@ -3233,6 +3295,7 @@ MpegTSContext *avpriv_mpegts_parse_open(AVFormatContext *s)
> >      ts->auto_guess = 1;
> >      mpegts_open_section_filter(ts, SDT_PID, sdt_cb, ts, 1);
> >      mpegts_open_section_filter(ts, PAT_PID, pat_cb, ts, 1);
> > +    mpegts_open_section_filter(ts, EIT_PID, eit_cb, ts, 1);
> >
> >      return ts;
> >  }
> > --
> > 2.23.0
>
> Regards,
> Marton
> _______________________________________________
> 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".

On Wed, 4 Sep 2019, Anthony Delannoy wrote:

> Hi
>
> I'm still interested to have those three commits merged (update in attachments).

Ok, below are some more comments.

>
> But I'd like to see data decoder in the future to use more easily
> EPG/NIT/BAT etc tables.
> Will it be possible? With modifications if it needs to be?

I don't see how, as it does not fit into the concept of the libav* 
libraries. I feel this belongs to a separate library.

> From 335b0bf377c1e5cfc5207561adc9621b113759b0 Mon Sep 17 00:00:00 2001
> From: Anthony Delannoy <anthony.2lannoy@gmail.com>
> Date: Wed, 21 Aug 2019 11:46:56 +0200
> Subject: [PATCH 3/3] lavf/mpegts: EPG extraction from mpegts
> 
> ---
>  libavformat/mpegts.c | 71 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 69 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index 0415ceea02..9bb6b6add8 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -168,6 +168,8 @@ struct MpegTSContext {
>      /** filters for various streams specified by PMT + for the PAT and PMT */
>      MpegTSFilter *pids[NB_PID_MAX];
>      int current_pid;
> +
> +    AVStream *epg_stream;
>  };
>
>  #define MPEGTS_OPTIONS \
> @@ -2498,13 +2500,68 @@ static void pat_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
>      }
>  }
> 
> +static void eit_cb(MpegTSFilter *filter, const uint8_t *section, int section_len)
> +{
> +    MpegTSContext *ts = filter->u.section_filter.opaque;
> +    const uint8_t *p, *p_end;
> +    SectionHeader h1, *h = &h1;
> +
> +    /*
> +     * Sometimes we receive EPG packets but SDT table do not have
> +     * eit_pres_following or eit_sched turned on, so we open EPG
> +     * stream directly here.
> +     */
> +    if (!ts->epg_stream) {
> +        ts->epg_stream = avformat_new_stream(ts->stream, NULL);
> +        if (!ts->epg_stream)
> +            return;
> +        ts->epg_stream->id = EIT_PID;
> +        ts->epg_stream->codecpar->codec_type = AVMEDIA_TYPE_DATA;
> +        ts->epg_stream->codecpar->codec_id = AV_CODEC_ID_EPG;
> +    }
> +
> +    if (ts->epg_stream->discard == AVDISCARD_ALL)
> +        return;
> +
> +    p_end = section + section_len - 4;
> +    p     = section;
> +
> +    if (parse_section_header(h, &p, p_end) < 0)
> +        return;
> +    if (h->tid < EIT_TID || h->tid > OEITS_END_TID)
> +        return;
> +
> +    av_log(ts->stream, AV_LOG_TRACE, "EIT: tid received = %.02x\n", h->tid);
> +
> +    /**
> +     * Service_id 0xFFFF is reserved, it indicates that the current EIT table
> +     * is scrambled.
> +     */
> +    if (h->id == 0xFFFF) {
> +        av_log(ts->stream, AV_LOG_WARNING, "Scrambled EIT table received.\n");
> +        return;
> +    }

In case of a scrambled EIT (which I have never seen myself in the wild) you'd
print this at every packet. You should either remove the warning, or check if
this is the first time (e.g. by checking if the EPG stream was just created).

> +
> +    /**
> +     * In case we receive an EPG packet before mpegts context is fully
> +     * initialized.
> +     */
> +    if (!ts->pkt)
> +        return;
> +
> +    new_data_packet(section, section_len, ts->pkt);
> +    ts->pkt->stream_index = ts->epg_stream->index;
> +    ts->stop_parse = 1;
> +}
> +
>  static void sdt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len)
>  {
>      MpegTSContext *ts = filter->u.section_filter.opaque;
>      MpegTSSectionFilter *tssf = &filter->u.section_filter;
>      SectionHeader h1, *h = &h1;
>      const uint8_t *p, *p_end, *desc_list_end, *desc_end;
> -    int onid, val, sid, desc_list_len, desc_tag, desc_len, service_type;
> +    int onid, val, sid, desc_list_len, desc_tag, desc_len, service_type,
> +        eit_sched, eit_pres_following;
>      char *name, *provider_name;
>
>      av_log(ts->stream, AV_LOG_TRACE, "SDT:\n");
> @@ -2534,6 +2591,13 @@ static void sdt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
>          val = get8(&p, p_end);
>          if (val < 0)
>              break;
> +        eit_sched = (val >> 1) & 0x1;
> +        eit_pres_following = val & 0x1;
> +
> +        if ((eit_sched | eit_pres_following) && !ts->epg_stream)
> +            av_log(ts->stream, AV_LOG_WARNING, "SDT table advertise EIT but no"
> +                   " packets were received yet.\n");
> +

You should remove this, there are tons of captures where EIT PID is
intentionally filtered, we should not spam the user.

>          desc_list_len = get16(&p, p_end);
>          if (desc_list_len < 0)
>              break;
> @@ -2984,8 +3048,8 @@ static int mpegts_read_header(AVFormatContext *s)
>          seek_back(s, pb, pos);
>
>          mpegts_open_section_filter(ts, SDT_PID, sdt_cb, ts, 1);
> -
>          mpegts_open_section_filter(ts, PAT_PID, pat_cb, ts, 1);
> +        mpegts_open_section_filter(ts, EIT_PID, eit_cb, ts, 1);
>
>          handle_packets(ts, probesize / ts->raw_packet_size);
>          /* if could not find service, enable auto_guess */
> @@ -3240,8 +3304,11 @@ MpegTSContext *avpriv_mpegts_parse_open(AVFormatContext *s)
>      ts->raw_packet_size = TS_PACKET_SIZE;
>      ts->stream = s;
>      ts->auto_guess = 1;
> +    ts->epg_stream = NULL;

Not needed, as context is zero initialized.

> +
>      mpegts_open_section_filter(ts, SDT_PID, sdt_cb, ts, 1);
>      mpegts_open_section_filter(ts, PAT_PID, pat_cb, ts, 1);
> +    mpegts_open_section_filter(ts, EIT_PID, eit_cb, ts, 1);
>
>      return ts;
>  }
> -- 
> 2.23.0

Regards,
Marton

On Sat, Sep 7, 2019 at 11:39 AM Marton Balint <cus@passwd.hu> wrote:

>
>
> On Wed, 4 Sep 2019, Anthony Delannoy wrote:
>
> > Hi
> >
> > I'm still interested to have those three commits merged (update in
> attachments).
>
> Ok, below are some more comments.
>
> >
> > But I'd like to see data decoder in the future to use more easily
> > EPG/NIT/BAT etc tables.
> > Will it be possible? With modifications if it needs to be?
>
> I don't see how, as it does not fit into the concept of the libav*
> libraries. I feel this belongs to a separate library.
>
> > From 335b0bf377c1e5cfc5207561adc9621b113759b0 Mon Sep 17 00:00:00 2001
> > From: Anthony Delannoy <anthony.2lannoy@gmail.com>
> > Date: Wed, 21 Aug 2019 11:46:56 +0200
> > Subject: [PATCH 3/3] lavf/mpegts: EPG extraction from mpegts
> >
> > ---
> >  libavformat/mpegts.c | 71 ++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 69 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index 0415ceea02..9bb6b6add8 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -168,6 +168,8 @@ struct MpegTSContext {
> >      /** filters for various streams specified by PMT + for the PAT and
> PMT */
> >      MpegTSFilter *pids[NB_PID_MAX];
> >      int current_pid;
> > +
> > +    AVStream *epg_stream;
> >  };
> >
> >  #define MPEGTS_OPTIONS \
> > @@ -2498,13 +2500,68 @@ static void pat_cb(MpegTSFilter *filter, const
> uint8_t *section, int section_len
> >      }
> >  }
> >
> > +static void eit_cb(MpegTSFilter *filter, const uint8_t *section, int
> section_len)
> > +{
> > +    MpegTSContext *ts = filter->u.section_filter.opaque;
> > +    const uint8_t *p, *p_end;
> > +    SectionHeader h1, *h = &h1;
> > +
> > +    /*
> > +     * Sometimes we receive EPG packets but SDT table do not have
> > +     * eit_pres_following or eit_sched turned on, so we open EPG
> > +     * stream directly here.
> > +     */
> > +    if (!ts->epg_stream) {
> > +        ts->epg_stream = avformat_new_stream(ts->stream, NULL);
> > +        if (!ts->epg_stream)
> > +            return;
> > +        ts->epg_stream->id = EIT_PID;
> > +        ts->epg_stream->codecpar->codec_type = AVMEDIA_TYPE_DATA;
> > +        ts->epg_stream->codecpar->codec_id = AV_CODEC_ID_EPG;
> > +    }
> > +
> > +    if (ts->epg_stream->discard == AVDISCARD_ALL)
> > +        return;
> > +
> > +    p_end = section + section_len - 4;
> > +    p     = section;
> > +
> > +    if (parse_section_header(h, &p, p_end) < 0)
> > +        return;
> > +    if (h->tid < EIT_TID || h->tid > OEITS_END_TID)
> > +        return;
> > +
> > +    av_log(ts->stream, AV_LOG_TRACE, "EIT: tid received = %.02x\n",
> h->tid);
> > +
> > +    /**
> > +     * Service_id 0xFFFF is reserved, it indicates that the current EIT
> table
> > +     * is scrambled.
> > +     */
> > +    if (h->id == 0xFFFF) {
> > +        av_log(ts->stream, AV_LOG_WARNING, "Scrambled EIT table
> received.\n");
> > +        return;
> > +    }
>
> In case of a scrambled EIT (which I have never seen myself in the wild)
> you'd
> print this at every packet. You should either remove the warning, or check
> if
> this is the first time (e.g. by checking if the EPG stream was just
> created).
>

I have a sample if anyone is interested. AFAIK it is only used by HD BBC
broadcasts in the UK.

https://tmm1.s3.amazonaws.com/dvb-eit/uk-eit-hd.ts

https://stackoverflow.com/questions/37765956/linux-dvb-v5-library-example-code-for-eit-grabbing#comment66609009_37765956


>
> > +
> > +    /**
> > +     * In case we receive an EPG packet before mpegts context is fully
> > +     * initialized.
> > +     */
> > +    if (!ts->pkt)
> > +        return;
> > +
> > +    new_data_packet(section, section_len, ts->pkt);
> > +    ts->pkt->stream_index = ts->epg_stream->index;
> > +    ts->stop_parse = 1;
> > +}
> > +
> >  static void sdt_cb(MpegTSFilter *filter, const uint8_t *section, int
> section_len)
> >  {
> >      MpegTSContext *ts = filter->u.section_filter.opaque;
> >      MpegTSSectionFilter *tssf = &filter->u.section_filter;
> >      SectionHeader h1, *h = &h1;
> >      const uint8_t *p, *p_end, *desc_list_end, *desc_end;
> > -    int onid, val, sid, desc_list_len, desc_tag, desc_len, service_type;
> > +    int onid, val, sid, desc_list_len, desc_tag, desc_len, service_type,
> > +        eit_sched, eit_pres_following;
> >      char *name, *provider_name;
> >
> >      av_log(ts->stream, AV_LOG_TRACE, "SDT:\n");
> > @@ -2534,6 +2591,13 @@ static void sdt_cb(MpegTSFilter *filter, const
> uint8_t *section, int section_len
> >          val = get8(&p, p_end);
> >          if (val < 0)
> >              break;
> > +        eit_sched = (val >> 1) & 0x1;
> > +        eit_pres_following = val & 0x1;
> > +
> > +        if ((eit_sched | eit_pres_following) && !ts->epg_stream)
> > +            av_log(ts->stream, AV_LOG_WARNING, "SDT table advertise EIT
> but no"
> > +                   " packets were received yet.\n");
> > +
>
> You should remove this, there are tons of captures where EIT PID is
> intentionally filtered, we should not spam the user.
>
> >          desc_list_len = get16(&p, p_end);
> >          if (desc_list_len < 0)
> >              break;
> > @@ -2984,8 +3048,8 @@ static int mpegts_read_header(AVFormatContext *s)
> >          seek_back(s, pb, pos);
> >
> >          mpegts_open_section_filter(ts, SDT_PID, sdt_cb, ts, 1);
> > -
> >          mpegts_open_section_filter(ts, PAT_PID, pat_cb, ts, 1);
> > +        mpegts_open_section_filter(ts, EIT_PID, eit_cb, ts, 1);
> >
> >          handle_packets(ts, probesize / ts->raw_packet_size);
> >          /* if could not find service, enable auto_guess */
> > @@ -3240,8 +3304,11 @@ MpegTSContext
> *avpriv_mpegts_parse_open(AVFormatContext *s)
> >      ts->raw_packet_size = TS_PACKET_SIZE;
> >      ts->stream = s;
> >      ts->auto_guess = 1;
> > +    ts->epg_stream = NULL;
>
> Not needed, as context is zero initialized.
>
> > +
> >      mpegts_open_section_filter(ts, SDT_PID, sdt_cb, ts, 1);
> >      mpegts_open_section_filter(ts, PAT_PID, pat_cb, ts, 1);
> > +    mpegts_open_section_filter(ts, EIT_PID, eit_cb, ts, 1);
> >
> >      return ts;
> >  }
> > --
> > 2.23.0
>
> Regards,
> Marton
> _______________________________________________
> 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".