[FFmpeg-devel] [PATCH v2] libavformat/mpegts: demux DVB VBI data

[Scott Theisen]

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.

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.

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.
"

Regards,

Scott Theisen