Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows
> -Original Message- > From: ffmpeg-devel On Behalf Of > Martin Storsjö > Sent: Sunday, May 8, 2022 10:12 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object > sharing issue on Windows > > On Sat, 7 May 2022, Soft Works wrote: > > >> -Original Message- > >> From: ffmpeg-devel On Behalf Of > >> Andreas Rheinhardt > >> Sent: Saturday, May 7, 2022 6:32 AM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object > >> sharing issue on Windows > >> > >> Soft Works: > >>> > >>> > -Original Message- > >>> > >>> I don't have experience with that kind of setup, but I would have > >>> thought that with separate CRTs, you could already get into > trouble > >>> when you would allocate a string in the main application which > >>> you pass to any of the DLL's APIs and which might get freed by > >>> the DLL at a later time - doesn't that fail? > >>> > >> > >> Whenever any of the FFmpeg libraries takes ownership of a string or > >> another buffer*, we require it to be freeable with av_free > (typically > >> by > >> saying that it needs to be allocated with the av_malloc family of > >> functions). So all allocs and frees have to happen in libavutil. > This > >> is > >> also true for all the other allocations directly performed by the > the > >> FFmpeg libraries. > >> (The only exceptions to this are AVBuffer(Ref)s which allow users > to > >> use > >> custom allocators and destructors.) > > > > Ah yes of course, thanks for the explanation. I still wonder whether > > there aren't any other issues when multiple CRTs are being used? > > > > Or are the file IO APIs the only "weak" point with regards to > > multiple CRTs being used? > > In the case of ffmpeg, yes. > > For generic library design, you'd have an issue anywhere where you > pass > CRT resources around - file descriptors from open, FILE*, and indeed > as > you mentioned - allocating and freeing memory with malloc/free in > different DLLs. But as long as the library design is such that you > don't > hand over ownership of allocations and don't pass such objects across > DLL > boundaries, there's no issue. Yup, understood. I thought there would be many more, but I realized that those "many more" I thought about are all C++ things, not C. So, putting this all together, I agree that the existence of av_fopen_utf8 (as a public API!) is rather unfortunate. To make this consistent, it would be necessary to provide av_ equivalents to all the file APIs as well (but there are quite a few). So I wonder whether it wouldn't make sense to deprecate this as a public API member? Kind regards, softworkz ___ 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".
Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows
> -Original Message- > From: ffmpeg-devel On Behalf Of > Martin Storsjö > Sent: Sunday, May 8, 2022 10:02 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object > sharing issue on Windows > > On Sat, 7 May 2022, Soft Works wrote: > > >> -Original Message- > >> From: ffmpeg-devel On Behalf Of > >> Martin Storsjö > >> Sent: Wednesday, April 20, 2022 2:48 PM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object > sharing > >> issue on Windows > >> > >> Hi, > >> > >> I just became aware of the av_fopen_utf8 function - which was > >> introduced > >> to fix path name translations on Windows - actually has a notable > >> design > >> flaw. > > > > Hi Martin, > > > > I just became aware that somebody would be compiling ffmpeg like > > this on Windows and I'm curious regarding the whereabouts.. > > > >> Background: > >> > >> On Windows, a process can contain more than one C runtime (CRT); > the > >> system comes with two shared ones (UCRT and msvcrt.dll) and in MSVC > >> builds, each DLL/EXE can have one statically linked in instead of > >> linking > >> against a shared library CRT (and that's actually the default > >> configuration when building with MSVC). > > > > The default configuration for both, EXE and DLL projects is to link > > to the C runtime dynamically (crt dll). > > No, that's not true. If you invoke e.g. "cl.exe myprog.c" without > explicitly passing either -MD or -MT, the default is -MT, which is to > statically link the CRT. What I meant is when you create a new console project in Visual Studio, the default is /MD, and the same is true when creating a new dll project. > >> This means that CRT objects (file descriptors from open(), FILE* > >> opened > >> with fopen/fdopen) mustn't be shared across DLLs; such an object > must > >> be > >> opened, accessed and closed within the same DLL. > > > > This only happens when you explicitly modify the build configuration > > to statically link to the CRT. > > No, this is not a custom build configuration. This is the build > configuration you get if you configure with "configure --enable-shared > --toolchain=msvc". Ok, then this is what needs to be fixed. When you configure for "shared", the exe and dll binaries need to be all compiled with /MD. > > Why are you compiling it this way? > > Your earlier patch is from 2013, so you seem to be doing so for > > quite a while. > > It's not that I'm shipping a production setup built this way. I just > spent > a fair amount of work to make ffmpeg work when built with MSVC; both > built > as static libraries and as DLLs. And I'd like to keep that working. > Not > only on the "seems to work for whatever is covered by fate" level, but > also on the level of not using constructs that are known to not work. > > As av_fopen_utf8 gets duplicated across the libraries by being in the > same > source file as the other functions, it works for all uses across the > libraries, but doesn't work for uses outside of the libraries > (fftools, > external API users). That's why I'm hesitant against increasing the > use of > this function in fftools until we have resolve this issue. Yes I agree to that. I will retract this part of my patchset. > Also, another fairly common situation where the "different CRTs" > scenario > happens if you'd e.g. build the ffmpeg libraries as DLLs with mingw, > but > then link against those DLLs with a user application built with MSVC. AFAIK, it is possible to create DLLs with mingw/MSYS2 in a way that these can link to a specific version of the MS CRT, but that's just a side note. > > Is the file API the only case where you had any trouble? > > As far as I remember, that was the only case of cross-library resource > sharing issue I ran into at the time. I'll follow up on this in your other reply. Thanks, softworkz ___ 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".
Re: [FFmpeg-devel] [PATCH 1/2] fftools: use av_fopen_utf8() instead of plain fopen()
> -Original Message- > From: ffmpeg-devel On Behalf Of > Martin Storsjö > Sent: Sunday, May 8, 2022 10:03 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 1/2] fftools: use av_fopen_utf8() > instead of plain fopen() > > On Sat, 7 May 2022, softworkz wrote: > > > From: softworkz > > > > Signed-off-by: softworkz > > --- > > fftools/cmdutils.c | 6 +++--- > > fftools/ffmpeg.c | 4 ++-- > > fftools/opt_common.c | 2 +- > > 3 files changed, 6 insertions(+), 6 deletions(-) > > Just for clarity (for someone looking at this individual mail thread > on > its own); please don't do this until the issue pointed out in > https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295488.html has > been > resolved. I agree. I will retract this part of the patchset for now. Thanks, softworkz ___ 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".
Re: [FFmpeg-devel] [PATCH] avformat/mov: fix timecode with rounded down tmcd nb_frames
On Sun, 1 May 2022, Marton Balint wrote: Regression since 8dd5bb728038f21d17ec789e21d65fe8f3f364a6. Fixes ticket #5978. Will apply tomorrow. Regards, Marton Signed-off-by: Marton Balint --- libavformat/mov.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 0f63d997fc..dfd5720bf4 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -8038,12 +8038,13 @@ static int mov_read_timecode_track(AVFormatContext *s, AVStream *st) int64_t cur_pos = avio_tell(sc->pb); int64_t value; AVRational tc_rate = st->avg_frame_rate; +int tmcd_nb_frames = sc->tmcd_nb_frames; int rounded_tc_rate; if (!sti->nb_index_entries) return -1; -if (!tc_rate.num || !tc_rate.den || !sc->tmcd_nb_frames) +if (!tc_rate.num || !tc_rate.den || !tmcd_nb_frames) return -1; avio_seek(sc->pb, sti->index_entries->pos, SEEK_SET); @@ -8060,9 +8061,15 @@ static int mov_read_timecode_track(AVFormatContext *s, AVStream *st) * format). */ /* 60 fps content have tmcd_nb_frames set to 30 but tc_rate set to 60, so - * we multiply the frame number with the quotient. */ + * we multiply the frame number with the quotient. + * See tickets #9492, #9710. */ rounded_tc_rate = (tc_rate.num + tc_rate.den / 2) / tc_rate.den; -value = av_rescale(value, rounded_tc_rate, sc->tmcd_nb_frames); +/* Work around files where tmcd_nb_frames is rounded down from frame rate + * instead of up. See ticket #5978. */ +if (tmcd_nb_frames == tc_rate.num / tc_rate.den && +s->strict_std_compliance < FF_COMPLIANCE_STRICT) +tmcd_nb_frames = rounded_tc_rate; +value = av_rescale(value, rounded_tc_rate, tmcd_nb_frames); parse_timecode_in_framenum_format(s, st, value, flags); -- 2.34.1 ___ 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". ___ 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".
Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows
On Sat, 7 May 2022, Soft Works wrote: -Original Message- From: ffmpeg-devel On Behalf Of Andreas Rheinhardt Sent: Saturday, May 7, 2022 6:32 AM To: ffmpeg-devel@ffmpeg.org Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows Soft Works: -Original Message- I don't have experience with that kind of setup, but I would have thought that with separate CRTs, you could already get into trouble when you would allocate a string in the main application which you pass to any of the DLL's APIs and which might get freed by the DLL at a later time - doesn't that fail? Whenever any of the FFmpeg libraries takes ownership of a string or another buffer*, we require it to be freeable with av_free (typically by saying that it needs to be allocated with the av_malloc family of functions). So all allocs and frees have to happen in libavutil. This is also true for all the other allocations directly performed by the the FFmpeg libraries. (The only exceptions to this are AVBuffer(Ref)s which allow users to use custom allocators and destructors.) Ah yes of course, thanks for the explanation. I still wonder whether there aren't any other issues when multiple CRTs are being used? Or are the file IO APIs the only "weak" point with regards to multiple CRTs being used? In the case of ffmpeg, yes. For generic library design, you'd have an issue anywhere where you pass CRT resources around - file descriptors from open, FILE*, and indeed as you mentioned - allocating and freeing memory with malloc/free in different DLLs. But as long as the library design is such that you don't hand over ownership of allocations and don't pass such objects across DLL boundaries, there's no issue. // Martin ___ 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".
Re: [FFmpeg-devel] [PATCH 1/2] fftools: use av_fopen_utf8() instead of plain fopen()
On Sat, 7 May 2022, softworkz wrote: From: softworkz Signed-off-by: softworkz --- fftools/cmdutils.c | 6 +++--- fftools/ffmpeg.c | 4 ++-- fftools/opt_common.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) Just for clarity (for someone looking at this individual mail thread on its own); please don't do this until the issue pointed out in https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295488.html has been resolved. // Martin ___ 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".
Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows
On Sat, 7 May 2022, Soft Works wrote: -Original Message- From: ffmpeg-devel On Behalf Of Martin Storsjö Sent: Wednesday, April 20, 2022 2:48 PM To: ffmpeg-devel@ffmpeg.org Subject: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows Hi, I just became aware of the av_fopen_utf8 function - which was introduced to fix path name translations on Windows - actually has a notable design flaw. Hi Martin, I just became aware that somebody would be compiling ffmpeg like this on Windows and I'm curious regarding the whereabouts.. Background: On Windows, a process can contain more than one C runtime (CRT); the system comes with two shared ones (UCRT and msvcrt.dll) and in MSVC builds, each DLL/EXE can have one statically linked in instead of linking against a shared library CRT (and that's actually the default configuration when building with MSVC). The default configuration for both, EXE and DLL projects is to link to the C runtime dynamically (crt dll). No, that's not true. If you invoke e.g. "cl.exe myprog.c" without explicitly passing either -MD or -MT, the default is -MT, which is to statically link the CRT. This means that CRT objects (file descriptors from open(), FILE* opened with fopen/fdopen) mustn't be shared across DLLs; such an object must be opened, accessed and closed within the same DLL. This only happens when you explicitly modify the build configuration to statically link to the CRT. No, this is not a custom build configuration. This is the build configuration you get if you configure with "configure --enable-shared --toolchain=msvc". Why are you compiling it this way? Your earlier patch is from 2013, so you seem to be doing so for quite a while. It's not that I'm shipping a production setup built this way. I just spent a fair amount of work to make ffmpeg work when built with MSVC; both built as static libraries and as DLLs. And I'd like to keep that working. Not only on the "seems to work for whatever is covered by fate" level, but also on the level of not using constructs that are known to not work. As av_fopen_utf8 gets duplicated across the libraries by being in the same source file as the other functions, it works for all uses across the libraries, but doesn't work for uses outside of the libraries (fftools, external API users). That's why I'm hesitant against increasing the use of this function in fftools until we have resolve this issue. Also, another fairly common situation where the "different CRTs" scenario happens if you'd e.g. build the ffmpeg libraries as DLLs with mingw, but then link against those DLLs with a user application built with MSVC. Then it's not an issue with the function between the libraries, but it is an issue for use of av_fopen_utf8 in the user's code outside of libav*. Is the file API the only case where you had any trouble? As far as I remember, that was the only case of cross-library resource sharing issue I ran into at the time. I don't have experience with that kind of setup, but I would have thought that with separate CRTs, you could already get into trouble when you would allocate a string in the main application which you pass to any of the DLL's APIs and which might get freed by the DLL at a later time - doesn't that fail? No, it doesn't (as Andreas explained). // Martin ___ 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".
Re: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
On Sat, 7 May 2022, Soft Works wrote: -Original Message- From: ffmpeg-devel On Behalf Of nil- admir...@mailo.com Sent: Friday, May 6, 2022 6:08 PM To: ffmpeg-devel@ffmpeg.org Subject: Re: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi As a matter of fact, you are. Your alternative method implies ploughing through hundreds of C files normalising path handling across the entire application, Almost everybody here knows that this isn't true. I do not. During the review of just this particular patchset it was found that FFmpeg sometimes uses av_fopen_utf8 and sometimes just plain fopen. Plain fopens were already replaced with av_fopen_utf8 and then reverted back. because suddenly it turned out that av_fopen_utf8 is problematic: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295488.html. Read again. As each lib gets its own copy of file_open.c there's no problem using av_fopen_utf8. The concern in that message was about it being a public API that could be used by external callers. No, it's not quite as simple. Yes, each library gets its own copy of av_fopen_utf8, but fftools doesn't get its own copy. That's why it's a real problem. (Secondly, one part of the trick of giving each library its own copy of is that avpriv_open is renamed ff_open, so that it is not an exported function. But in the case of av_fopen_utf8, it's an entirely public API, so we can't hide it.) // Martin ___ 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".
Re: [FFmpeg-devel] [PATCH v3 06/11] libavformat/asfdec: remove unused parameters
On Sun, May 08, 2022 at 03:01:17AM +, softworkz wrote: > From: softworkz > > Signed-off-by: softworkz > --- > libavformat/asfdec_f.c | 30 +++--- > 1 file changed, 15 insertions(+), 15 deletions(-) will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. signature.asc Description: PGP signature ___ 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".
Re: [FFmpeg-devel] [PATCH 3/3] avformat/mov: fix use invalid box size/type due to eof
> On Apr 26, 2022, at 4:20 PM, Zhao Zhili wrote:
>
> ---
> libavformat/mov.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index e948c6cd0f..429f9fcbf7 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -7691,10 +7691,12 @@ static int mov_read_default(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
>
> if (atom.size < 0)
> atom.size = INT64_MAX;
> -while (total_size <= atom.size - 8 && !avio_feof(pb)) {
> +while (total_size <= atom.size - 8) {
> int (*parse)(MOVContext*, AVIOContext*, MOVAtom) = NULL;
> a.size = avio_rb32(pb);
> a.type = avio_rl32(pb);
> +if (avio_feof(pb))
> +break;
> if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) ||
> a.type == MKTAG('h','o','o','v')) &&
> a.size >= 8 &&
> --
Applied.
___
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".
Re: [FFmpeg-devel] [PATCH v2 1/2] avformat/hlsenc: check discont_program_date_time before use it in parse_playlist
> On May 8, 2022, at 9:17 AM, Steven Liu wrote:
>
> In parse_playlist, the discont_program_date_time should be used after
> EXT-X-PROGRAM-DATE-TIME tag parsed.
>
> Signed-off-by: Steven Liu
> ---
> libavformat/hlsenc.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index d2b8215dff..b9f79e30d8 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -1288,8 +1288,10 @@ static int parse_playlist(AVFormatContext *s, const
> char *url, VariantStream *vs
> new_start_pos = avio_tell(vs->avf->pb);
> vs->size = new_start_pos - vs->start_pos;
> ret = hls_append_segment(s, hls, vs, vs->duration,
> vs->start_pos, vs->size);
> -vs->last_segment->discont_program_date_time =
> discont_program_date_time;
> -discont_program_date_time += vs->duration;
> +if (discont_program_date_time) {
> +vs->last_segment->discont_program_date_time =
> discont_program_date_time;
> +discont_program_date_time += vs->duration;
> +}
Looks good to me, although it doesn’t work if EXT-X-PROGRAM-DATE-TIME is
1970-01-01 :)
> if (ret < 0)
> goto fail;
> vs->start_pos = new_start_pos;
> --
> 2.34.1
>
> ___
> 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".
___
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".
Re: [FFmpeg-devel] [PATCH v2 2/2] avformat/hlsenc: expand the scope of flags from int32_t to int64_t
> On May 8, 2022, at 9:17 AM, Steven Liu wrote:
>
> because the flags in AVOption support i64.
>
> Signed-off-by: Steven Liu
> ---
> libavformat/hlsenc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index b9f79e30d8..00645ae74e 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -202,7 +202,7 @@ typedef struct HLSContext {
> int64_t init_time; // Set by a private option.
> int max_nb_segments; // Set by a private option.
> int hls_delete_threshold; // Set by a private option.
> -uint32_t flags;// enum HLSFlags
> +uint64_t flags;// enum HLSFlags
Actually it should be int here.
Firstly, AV_OPT_TYPE_FLAGS is accessed as int in opt.c.
uint64_t only works with little endian system.
Secondly, enum HLSFlags cannot has value larger than int.
> uint32_t pl_type; // enum PlaylistType
> char *segment_filename;
> char *fmp4_init_filename;
> --
> 2.34.1
>
> ___
> 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".
___
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".
[FFmpeg-devel] [PATCH 2/2] avcodec: add an AVCodecContext flag to export A53/SCTE20/DVD CC side data on demand
From: Limin Wang
some samples include both A53 and SCTE20 data. Before the commit, both
of the will be exported, so the CC data will be repeated or garbarge
as they're using the same frame side data. If you know your samples include
only one of them, You can export by +a53cc+scte20.
After the commit, the default will not export MPEG2 A53/SCTE20/DVD CC side data,
please export on demand.
Signed-off-by: Limin Wang
---
doc/codecs.texi| 10 ++
libavcodec/avcodec.h | 16 +++-
libavcodec/mpeg12dec.c | 6 +++---
libavcodec/options_table.h | 3 +++
libavcodec/version.h | 2 +-
tests/fate/ffmpeg.mak | 2 +-
tests/fate/subtitles.mak | 6 +++---
7 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/doc/codecs.texi b/doc/codecs.texi
index 5e10020900..4cced983b9 100644
--- a/doc/codecs.texi
+++ b/doc/codecs.texi
@@ -662,6 +662,16 @@ for codecs that support it. At present, those are H.264
and VP9.
@item film_grain
Export film grain parameters through frame side data (see
@code{AV_FRAME_DATA_FILM_GRAIN_PARAMS}).
Supported at present by AV1 decoders.
+@item a53cc
+Export A53 CC through frame side data (see @code{AV_FRAME_DATA_A53_CC})
+for codecs that support it.
+@item scte20cc
+Export SCTE20 CC through frame side data (see @code{AV_FRAME_DATA_A53_CC})
+for codecs that support it.
+@item dvdcc
+Export DVD CC through frame side data (see @code{AV_FRAME_DATA_A53_CC})
+for codecs that support it.
+
@end table
@item threads @var{integer} (@emph{decoding/encoding,video})
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 4dae23d06e..25fd4de2fe 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -360,7 +360,21 @@ typedef struct RcOverride{
* Do not apply film grain, export it instead.
*/
#define AV_CODEC_EXPORT_DATA_FILM_GRAIN (1 << 3)
-
+/**
+ * Decoding only.
+ * Export A53 CC through frame side data
+ */
+#define AV_CODEC_EXPORT_DATA_A53_CC (1 << 4)
+/**
+ * Decoding only.
+ * Export SCTE20 CC through frame side data
+ */
+#define AV_CODEC_EXPORT_DATA_SCTE20_CC (1 << 5)
+/**
+ * Decoding only.
+ * Export DVD CC through frame side data
+ */
+#define AV_CODEC_EXPORT_DATA_DVD_CC (1 << 6)
/**
* The decoder will keep a reference to the frame and may reuse it later.
*/
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index e9bde48f7a..032cb8f9b1 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2203,7 +2203,7 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx,
{
Mpeg1Context *s1 = avctx->priv_data;
-if (buf_size >= 6 &&
+if (buf_size >= 6 && (avctx->export_side_data &
AV_CODEC_EXPORT_DATA_A53_CC) &&
p[0] == 'G' && p[1] == 'A' && p[2] == '9' && p[3] == '4' &&
p[4] == 3 && (p[5] & 0x40)) {
/* extract A53 Part 4 CC data */
@@ -2224,7 +2224,7 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx,
avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
}
return 1;
-} else if (buf_size >= 2 &&
+} else if (buf_size >= 2 && (avctx->export_side_data &
AV_CODEC_EXPORT_DATA_SCTE20_CC) &&
p[0] == 0x03 && (p[1]&0x7f) == 0x01) {
/* extract SCTE-20 CC data */
GetBitContext gb;
@@ -2269,7 +2269,7 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx,
avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
}
return 1;
-} else if (buf_size >= 11 &&
+} else if (buf_size >= 11 && (avctx->export_side_data &
AV_CODEC_EXPORT_DATA_DVD_CC) &&
p[0] == 'C' && p[1] == 'C' && p[2] == 0x01 && p[3] == 0xf8) {
/* extract DVD CC data
*
diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index e72b4d12b6..3c6db07459 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -88,6 +88,9 @@ static const AVOption avcodec_options[] = {
{"prft", "export Producer Reference Time through packet side data", 0,
AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_PRFT}, INT_MIN, INT_MAX,
A|V|S|E, "export_side_data"},
{"venc_params", "export video encoding parameters through frame side data", 0,
AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS}, INT_MIN,
INT_MAX, V|D, "export_side_data"},
{"film_grain", "export film grain parameters through frame side data", 0,
AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_FILM_GRAIN}, INT_MIN, INT_MAX,
V|D, "export_side_data"},
+{"a53cc", "export A53 CC through frame side data", 0, AV_OPT_TYPE_CONST, {.i64
= AV_CODEC_EXPORT_DATA_A53_CC}, INT_MIN, INT_MAX, V|D, "export_side_data"},
+{"scte20cc", "export SCTE20 CC through frame side data", 0, AV_OPT_TYPE_CONST,
{.i64 = AV_CODEC_EXPORT_DATA_SCTE20_CC}, INT_MIN, INT_MAX, V|D,
"export_side_data"},
+{"dvdcc", "export DVD CC through frame side data", 0, AV_OPT_TYPE_CONST, {.i64
= AV_CODEC_EXPORT_DATA_DVD_CC}, INT_MIN, INT_MAX, V|D, "export_side_data"},
{"time_base", NULL, OFFSET(time_base),
[FFmpeg-devel] [PATCH 1/2] avfilter/src_movie: add dec_opts for the opened file
From: Limin Wang
Signed-off-by: Limin Wang
---
doc/filters.texi| 9 +
libavfilter/src_movie.c | 5 -
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 367614d2f8..6775cf43ba 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -29652,6 +29652,15 @@ shows how to add protocol_whitelist and
protocol_blacklist options:
ffplay -f lavfi
"movie=filename='1.sdp':format_opts='protocol_whitelist=file,rtp,udp\:protocol_blacklist=http'"
@end example
+
+@item dec_opts
+Specify decode options for the opened file. Format options can be specified
+as a list of @var{key}=@var{value} pairs separated by ':'. The following
example
+shows how to add export_side_data options:
+@example
+./ffmpeg -y -f lavfi
+-i "movie=./input.ts:dec_opts=export_side_data=scte20cc[out0+subcc]" out.srt
+@end example
@end table
It allows overlaying a second video on top of the main input of
diff --git a/libavfilter/src_movie.c b/libavfilter/src_movie.c
index 711854c23c..c7dbd90aa9 100644
--- a/libavfilter/src_movie.c
+++ b/libavfilter/src_movie.c
@@ -70,6 +70,7 @@ typedef struct MovieContext {
int64_t discontinuity_threshold;
int64_t ts_offset;
int dec_threads;
+AVDictionary *dec_opts;
AVFormatContext *format_ctx;
@@ -96,6 +97,7 @@ static const AVOption movie_options[]= {
{ "discontinuity", "set discontinuity threshold",
OFFSET(discontinuity_threshold), AV_OPT_TYPE_DURATION, {.i64 = 0}, 0,
INT64_MAX, FLAGS },
{ "dec_threads", "set the number of threads for decoding",
OFFSET(dec_threads), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, FLAGS },
{ "format_opts", "set format options for the opened file",
OFFSET(format_opts), AV_OPT_TYPE_DICT, {.str = NULL}, 0, 0, FLAGS},
+{ "dec_opts", "set decode options for the opened file",
OFFSET(dec_opts),AV_OPT_TYPE_DICT, {.str = NULL}, 0, 0, FLAGS},
{ NULL },
};
@@ -158,6 +160,7 @@ static AVStream *find_stream(void *log, AVFormatContext
*avf, const char *spec)
static int open_stream(AVFilterContext *ctx, MovieStream *st, int dec_threads)
{
+MovieContext *movie = ctx->priv;
const AVCodec *codec;
int ret;
@@ -179,7 +182,7 @@ static int open_stream(AVFilterContext *ctx, MovieStream
*st, int dec_threads)
dec_threads = ff_filter_get_nb_threads(ctx);
st->codec_ctx->thread_count = dec_threads;
-if ((ret = avcodec_open2(st->codec_ctx, codec, NULL)) < 0) {
+if ((ret = avcodec_open2(st->codec_ctx, codec, >dec_opts)) < 0) {
av_log(ctx, AV_LOG_ERROR, "Failed to open codec\n");
return ret;
}
--
2.35.1
___
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".
