| Message ID | CAB0OVGpAdfBgbC0A=zdjEZdRv_XphT=jMsJHp4_1cb7GiqnCWw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, 16 Feb 2019, Carl Eugen Hoyos wrote: > Hi! > > I am not sure why there is an option to disable Closed Captions export, > but disabling the export by default seems like a bad idea to me. SMPTE 436M can be any kind of ancillary data, not only closed captions. That is why the default behaviour (pass all the data to userspace so a userspace app can parse it properly) makes sense. Some applications might depend on this. The eia608_extract option is a hack which effectively replaces the generic ancillary data stream with the parsed subtitle stream. The reason for the hack is that libavcodec has no API for such a parser or filter which reads a data stream and generates one (ore more) subtitle streams... Regards, Marton
2019-02-17 1:40 GMT+01:00, Marton Balint <cus@passwd.hu>: > > On Sat, 16 Feb 2019, Carl Eugen Hoyos wrote: >> >> I am not sure why there is an option to disable Closed Captions export, >> but disabling the export by default seems like a bad idea to me. > > SMPTE 436M can be any kind of ancillary data, not only closed captions. > That is why the default behaviour (pass all the data to userspace so a > userspace app can parse it properly) makes sense. > Some applications might depend on this. Wouldn't it still make more sense to change the default? The application that knows it needs the ancillary data, well, knows it while the average user who reads the mxf file has no idea that there is a subtitle stream and has no idea that FFmpeg can read the subtitle stream. I would bump micro in any case and the option also works fine with older FFmpeg versions so I don't really see the issue. Carl Eugen
On Sun, 17 Feb 2019, Carl Eugen Hoyos wrote: > 2019-02-17 1:40 GMT+01:00, Marton Balint <cus@passwd.hu>: >> >> On Sat, 16 Feb 2019, Carl Eugen Hoyos wrote: >>> >>> I am not sure why there is an option to disable Closed Captions export, >>> but disabling the export by default seems like a bad idea to me. >> >> SMPTE 436M can be any kind of ancillary data, not only closed captions. >> That is why the default behaviour (pass all the data to userspace so a >> userspace app can parse it properly) makes sense. > >> Some applications might depend on this. > > Wouldn't it still make more sense to change the default? Sorry, no. We should not change existing behaviour for this. > The application that knows it needs the ancillary data, well, knows > it while the average user who reads the mxf file has no idea that > there is a subtitle stream and has no idea that FFmpeg can read > the subtitle stream. > > I would bump micro in any case and the option also works fine > with older FFmpeg versions so I don't really see the issue. Issue is that you are removing a data stream which was previously provided. Also CC extraction in MXF decoder is a hack. It should be deprecated after the API will be capable of bitstream filtering between different codecs. And the VANC data can be parsed to multiple subtitle/data streams. Why eia608 has the perference? Why not teletext, or SCTE messages? All in all, it is better to keep hacks under the option until we come up with better API. Regards, Marton
On Sun, 17 Feb 2019, Marton Balint wrote: > > > On Sun, 17 Feb 2019, Carl Eugen Hoyos wrote: > >> 2019-02-17 1:40 GMT+01:00, Marton Balint <cus@passwd.hu>: >>> >>> On Sat, 16 Feb 2019, Carl Eugen Hoyos wrote: >>>> >>>> I am not sure why there is an option to disable Closed Captions export, >>>> but disabling the export by default seems like a bad idea to me. >>> >>> SMPTE 436M can be any kind of ancillary data, not only closed captions. >>> That is why the default behaviour (pass all the data to userspace so a >>> userspace app can parse it properly) makes sense. >> >>> Some applications might depend on this. >> >> Wouldn't it still make more sense to change the default? > > Sorry, no. We should not change existing behaviour for this. > >> The application that knows it needs the ancillary data, well, knows >> it while the average user who reads the mxf file has no idea that >> there is a subtitle stream and has no idea that FFmpeg can read >> the subtitle stream. >> >> I would bump micro in any case and the option also works fine >> with older FFmpeg versions so I don't really see the issue. > > Issue is that you are removing a data stream which was previously > provided. Also CC extraction in MXF decoder is a hack. It should be > deprecated after the API will be capable of bitstream filtering between > different codecs. And the VANC data can be parsed to multiple > subtitle/data streams. Why eia608 has the perference? Why not teletext, or > SCTE messages? All in all, it is better to keep hacks under the > option until we come up with better API. One more thing is that the fact that a file has VANC data does not mean that it actually has Closed Captions data. It might, or it might not, so unconditionally advertising a CC stream just because there _might_ be CC data would also be bad. Regards, Marton
sön 2019-02-17 klockan 16:28 +0100 skrev Marton Balint: > > On Sun, 17 Feb 2019, Marton Balint wrote: > > > > > > > On Sun, 17 Feb 2019, Carl Eugen Hoyos wrote: > > > > > > > > 2019-02-17 1:40 GMT+01:00, Marton Balint <cus@passwd.hu>: > > > > > > > > On Sat, 16 Feb 2019, Carl Eugen Hoyos wrote: > > > > > > > > > > I am not sure why there is an option to disable Closed Captions export, > > > > > but disabling the export by default seems like a bad idea to me. > > > > > > > > SMPTE 436M can be any kind of ancillary data, not only closed captions. > > > > That is why the default behaviour (pass all the data to userspace so a > > > > userspace app can parse it properly) makes sense. > > > > Some applications might depend on this. > > > > > > Wouldn't it still make more sense to change the default? > > > > Sorry, no. We should not change existing behaviour for this. > > > > > The application that knows it needs the ancillary data, well, knows > > > it while the average user who reads the mxf file has no idea that > > > there is a subtitle stream and has no idea that FFmpeg can read > > > the subtitle stream. > > > > > > I would bump micro in any case and the option also works fine > > > with older FFmpeg versions so I don't really see the issue. > > > > Issue is that you are removing a data stream which was previously > > provided. Also CC extraction in MXF decoder is a hack. It should be > > deprecated after the API will be capable of bitstream filtering between > > different codecs. And the VANC data can be parsed to multiple > > subtitle/data streams. Why eia608 has the perference? Why not teletext, or > > SCTE messages? All in all, it is better to keep hacks under the > > option until we come up with better API. > > One more thing is that the fact that a file has VANC data does not mean > that it actually has Closed Captions data. It might, or it might not, so > unconditionally advertising a CC stream just because there _might_ be CC > data would also be bad. Isn't this stuff also used for teletext and whatnot? I generally agree that we shouldn't assume it's subtitles /Tomas
From 9ee6b52c22ae516f9f10116381123595ea51bb0a Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceffmpeg@gmail.com> Date: Sat, 16 Feb 2019 20:24:03 +0100 Subject: [PATCH] lavf/mxfdec: Extract EIA608 Closed Captions by default. Related to ticket #5362. --- libavformat/mxfdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index 0553adc..b6c9e60 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -3712,7 +3712,7 @@ static int mxf_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti static const AVOption options[] = { { "eia608_extract", "extract eia 608 captions from s436m track", - offsetof(MXFContext, eia608_extract), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, + offsetof(MXFContext, eia608_extract), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, AV_OPT_FLAG_DECODING_PARAM }, { NULL }, }; -- 1.7.10.4