On Sun, 8 Dec 2024, Scott Theisen wrote:

On 12/3/24 17:23, Marton Balint wrote:


 On Tue, 3 Dec 2024, Marton Balint wrote:



 On Sat, 30 Nov 2024, Scott Theisen wrote:

  DVB VBI data is defined in ETSI EN 301 775 and can include EBU teletext
  data
  as defined in ETSI EN 300 472.

  ETSI EN 300 468 defines teletext_descriptor, VBI_data_descriptor, and
  VBI_teletext_descriptor, which has the same definition as, but
 different
  use
  from, teletext_descriptor.
  ---
  libavcodec/codec_desc.c | 6 ++++++
  libavcodec/codec_id.h   | 1 +
  libavformat/mpegts.c    | 3 +++
  libavformat/mpegts.h    | 2 ++
  4 files changed, 12 insertions(+)

 You should split this to two patches.

OK, I'll do that.


 Patch 1 should add the codec ID the codec_description and please also
 update the assertion check in libavcodec/version.c.

 Patch 2 should add the support for demuxing in mpegts. I'd rather put the
 VBI descriptors after the teletext descriptor in the list, so in case
 multiple descriptors are present the detected codec should be teletext.

 On second thought the order does not help, because the codec will be
 determined by the first descriptor...

 Maybe when parsing the teletext descriptor it should override VBI codec to
 teletext, e.g.:

     case TELETEXT_DESCRIPTOR:
          if (codec_id == DVB_VBI)
                 codec_id = DVB_TELETEXT
          // fall-through
     case VBI_TELETEXT_DESCRIPTOR:
          ....

I'm not sure it makes sense to change it from DVB_VBI to DVB_TELETEXT, since the VBI format is backwards compatible with the teletext format.  Although, the TELETEXT_DESCRIPTOR is for EBU teletext data only streams, but then there shouldn't be either VBI descriptor to set the codec_id to DVB_VBI.

I'm not strongly opposed, I'm just not sure it is really necessary.

The reason I prefer it that way so streams which were detected earlier as teletext won't be suddenly detected as VBI because both descriptors are present and VBI is the first. I think using both descriptors is not unlikely, since the essence is compatible, here is an example I stumbled upon by accident:

https://www.dvbcontrol.com/wp-content/uploads/DVBAnalyzer/DVBAnalyzer_View_0.jpg


If you do want it, I'm not sure the if is necessary, just set codec_id unconditionally.  However, it might be better to just call mpegts_find_stream_type() instead so FFStream::need_context_update is set.

Yes, good point calling mpegts_find_stream_type() instead.


Regardless, I'm wondering if instead VBI_TELETEXT_DESCRIPTOR should not be added to DESC_types since ETSI EN 300 468 says this about VBI_teletext_descriptor:
"
The only exception is that the VBI_teletext_descriptor is not to be used to associate stream_type 0x06 with the VBI standard nor the EBU teletext standard. Decoders can only use the languages in this descriptor to select magazines and subtitles.
"


Yeah, we could remove it from DESC_types if we want to strictly follow the standard. However, as far as understand, a VBI_teletext_descriptor should never appear alone, only accompanied by a VBI_data_descriptor. So the question is if there are non-compliant streams with VBI_teletext_descriptor only which we want to support? I don't really have a preference here, so do whichever you think best.

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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to