[FFmpeg-devel] [PATCH V4 2/2] configure: replace 'pr' with printf since busybox does not support pr
It is part of change from https://trac.ffmpeg.org/ticket/5680 provided
by Kylie McClain at Wed, 29 Jun 2016 16:37:20 -0400.
That change contains two parts, in function log_file and in function
print_in_columns.
The second part is not good, so I have send out a new patch for
print_in_columns.
As for the change in the first part, it is good. Just to make the whole thing
completed,
i send out this patch to copy exactally the change in function log_file.
contributor: Kylie McClain
Signed-off-by: Guo, Yejun
---
configure | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 8941063..9d2d7be 100755
--- a/configure
+++ b/configure
@@ -503,7 +503,11 @@ log(){
log_file(){
log BEGIN $1
-pr -n -t $1 >> $logfile
+i=1
+while read line;do
+printf '%5s %s\n' "${i}" "${line}"
+i=$(($i+1))
+done < $1 >> $logfile
log END $1
}
--
2.7.4
___
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 V4 1/2] configure: sort decoder/encoder/filter/... names in alphabet order
take decoder names an example, with the default page length, shell command
'pr' needs two pages for all the decoder names. The names are firstly printed
in the first page, then in the second page. So, as a whole, the names are
sorted neither in column order nor in row order. It's a little confused.
One method is to calculate the proper page length, so all the names are printed
in one page by 'pr -l', and so strictly in alphabet order, column by column.
Another method is to use command printf instead of pr, because buybox doesn't
have pr. This patch refines print_in_columns to print the names with printf
in alphabet order, very similar with 'pr -l', except the case when the last
column is not fully filled with names.
contributor: Alexander Strasser
contributor: avih
Signed-off-by: Guo, Yejun
---
configure | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index 3b11ffe..8941063 100755
--- a/configure
+++ b/configure
@@ -3832,8 +3832,25 @@ die_unknown(){
}
print_in_columns() {
-cols=$(expr $ncols / 24)
-cat | tr ' ' '\n' | sort | pr -r "-$cols" -w $ncols -t
+set -- $(cat | tr ' ' '\n' | sort)
+col_width=24
+cols=$(($ncols / $col_width))
+rows=$(($(($# + $cols - 1)) / $cols))
+cols_seq=$(seq $cols)
+rows_seq=$(seq $rows)
+for row in $rows_seq; do
+index=$row
+line=""
+fmt=""
+for col in $cols_seq; do
+if [ $index -le $# ]; then
+eval line='"$line "${'$index'}'
+fmt="$fmt%-${col_width}s"
+fi
+index=$(($index + $rows))
+done
+printf "$fmt\n" $line
+done | sed 's/ *$//'
}
show_list() {
--
2.7.4
___
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 1/2] configure: sort decoder/encoder/filter/... names in alphabet order
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> avih
> Sent: Tuesday, April 23, 2019 8:14 PM
> To: FFmpeg development discussions and patches
> Cc: Guo, Yejun
> Subject: Re: [FFmpeg-devel] [PATCH V3 1/2] configure: sort
> decoder/encoder/filter/... names in alphabet order
>
> > print_in_columns() {
> > - cols=$(expr $ncols / 24)
> > - cat | tr ' ' '\n' | sort | pr -r "-$cols" -w $ncols -t
> > + col_width=24
> > + cols=$(expr $ncols / $col_width)
> > + rows=$(expr $(expr $# + $cols - 1) / $cols)
> > + for row in $(seq $rows); do
> > + index=$row
> > + line=""
> > + fmt=""
> > + for col in $(seq $cols); do
> > + if [ $index -le $# ]; then
> > + eval line='"$line "${'$index'}'
> > + fmt="$fmt%-${col_width}s"
> > + fi
> > + index=$(expr $index + $rows)
> > + done
> > + printf "$fmt\n" $line
> > + done | sed 's/ *$//'
> > }
>
>
> The new code is relatively slow.
>
> On linux it adds ~ 1.5s (1500 ms) to the configure time in both bash and dash
> on my system, which is roughly additional ~20% to configure run time.
> On Windows this will easily add a minute or more (I didn't test).
I tried with mingw on windows, the whole configure 'date;./configure;date' takes
about 2:25 (minute:second) with master, it takes about 3:41 with this v3 patch,
and
takes about 2:15 with v4 patch with your suggestions, v4 will be sent out next.
I also tried on ubuntu 16.04, the consumed times are: 8.5 seconds (master), 12
seconds (v3 patch), and 7.5 seconds (v4 patch).
>
>
> Few things to consider:
>
> - print_in_column iterates over a _lot_ of values - hundreds or more.
>
> - Subshells - `$(cmd ...)` are relatively very expensive, especially in hot
> (inner) loops, and especially on Windows.
>
> - $(expr ...) can typically be replaced with shell arithmetics - $((...)) .
> Your part 2 already uses it, and regardless it's been used in configure
> before (in pushvar and popvar), so you should use it where possible.
I just thought they are similar and so choose it by random.
Find the performance is better after change to $((...)).
>
> - `for col in $(seq $cols)` need not invoke `seq` on each iterations to always
> produce the same output. You can capture its output once on startup.
thanks, and it helps the performance too.
>
> - All the places which use the new print_in_columns want the result sorted and
> duplicate pipe through `tr` and `sort`. The original version already did
> `cat | tr ' ' '\n' | sort` in one place, and you can simply replace it with
> `set -- $(cat | tr ' ' '\n' | sort)` to get the values as positional
> parameters. This will also keep the interface the same as before and will
> reduce the patch size.
thanks, good point.
>
> And finally, is there a good reason to sort the results in columns and not
> rows?
> As you rightfully mentioned, outputs can span several pages, and when reading
> it on screen it might be more convenient to read row by row than column by
> column. This could also simplify the new code a lot.
To check a name with human eye, after we get the nearby area for the check:
If row by row, we have to turn our eyes from left to right, from up to down,
and then again left to right, up to down, maybe again and again. And there is
still
possibility that we missed the check.
If column by column, we just turn from up to down (the same column) and
can easily check if the name is there or not.
>
> ___
> 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] avformat/mpegenc - reject unsupported audio streams
On 24-04-2019 03:30 AM, Carl Eugen Hoyos wrote: 2019-04-22 13:00 GMT+02:00, Gyan : On 22-04-2019 01:15 PM, Gyan wrote: On 22-04-2019 12:30 PM, Carl Eugen Hoyos wrote: Am 20.04.2019 um 11:31 schrieb Gyan : Old patch that was never applied. Rebased. Please return patch_welcome for mlp and truehd. Will do. Wasn’t there another comment (not by me): “Why can’t .codec_tag be used?” There's no codec_tag member set. This patch is just to disallow an invalid muxing to proceed. Wasn't the argument that this is possible with codec_tag? Not in the present state. If you want to add a codec_tag, that's a separate patch. Revised. What about aac? There's no provision to mux AAC, at present. On demux, the stream is detected as mp2. And can't you wait more than a few hours for a review? This patch is 14 months old, except for the one change you requested. And all the patch does is enforce the existing limitations. Gyan ___ 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 2/4] cbs_mpeg2: Improve checks for invalid values
On 4/23/2019 7:55 PM, Andreas Rheinhardt wrote:
> horizontal/vertical_size_value (containing the twelve least significant
> bits of the frame size) mustn't be zero according to the specifications;
> and the value 0 is forbidden for the colour_description elements.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> The actual function calls after macro expansion are the same as in the
> earlier versions with the exception of the extra_bit_slice calls.
> libavcodec/cbs_mpeg2.c | 14 --
> libavcodec/cbs_mpeg2_syntax_template.c | 20 ++--
> 2 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index cdde68ea38..fc21745a51 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -41,20 +41,22 @@
> #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs,
> __VA_ARGS__ }) : NULL)
>
> #define ui(width, name) \
> -xui(width, name, current->name, 0)
> +xui(width, name, current->name, 0, MAX_UINT_BITS(width), 0)
> +#define uir(width, name, range_min, range_max) \
> +xui(width, name, current->name, range_min, range_max, 0)
> #define uis(width, name, subs, ...) \
> -xui(width, name, current->name, subs, __VA_ARGS__)
> +xui(width, name, current->name, 0, MAX_UINT_BITS(width), subs,
> __VA_ARGS__)
>
>
> #define READ
> #define READWRITE read
> #define RWContext GetBitContext
>
> -#define xui(width, name, var, subs, ...) do { \
> +#define xui(width, name, var, range_min, range_max, subs, ...) do { \
> uint32_t value = 0; \
> CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \
> SUBSCRIPTS(subs, __VA_ARGS__), \
> - , 0, (1 << width) - 1)); \
> + , range_min, range_max)); \
> var = value; \
> } while (0)
>
> @@ -81,10 +83,10 @@
> #define READWRITE write
> #define RWContext PutBitContext
>
> -#define xui(width, name, var, subs, ...) do { \
> +#define xui(width, name, var, range_min, range_max, subs, ...) do { \
> CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \
> SUBSCRIPTS(subs, __VA_ARGS__), \
> -var, 0, (1 << width) - 1)); \
> +var, range_min, range_max)); \
> } while (0)
>
> #define marker_bit() do { \
> diff --git a/libavcodec/cbs_mpeg2_syntax_template.c
> b/libavcodec/cbs_mpeg2_syntax_template.c
> index 10aaea7734..27dcaad316 100644
> --- a/libavcodec/cbs_mpeg2_syntax_template.c
> +++ b/libavcodec/cbs_mpeg2_syntax_template.c
> @@ -26,8 +26,8 @@ static int FUNC(sequence_header)(CodedBitstreamContext
> *ctx, RWContext *rw,
>
> ui(8, sequence_header_code);
>
> -ui(12, horizontal_size_value);
> -ui(12, vertical_size_value);
> +uir(12, horizontal_size_value, 1, 4095);
> +uir(12, vertical_size_value, 1, 4095);
>
> mpeg2->horizontal_size = current->horizontal_size_value;
> mpeg2->vertical_size = current->vertical_size_value;
> @@ -79,7 +79,7 @@ static int FUNC(user_data)(CodedBitstreamContext *ctx,
> RWContext *rw,
> #endif
>
> for (k = 0; k < current->user_data_length; k++)
> -xui(8, user_data, current->user_data[k], 0);
> +xui(8, user_data, current->user_data[k], 0, 255, 0);
Not sure why this is using xui() to being with, when it could simply be
uis(8, user_data[k], 1, k);
And it would also look better in the trace output.
>
> return 0;
> }
> @@ -125,9 +125,9 @@ static int
> FUNC(sequence_display_extension)(CodedBitstreamContext *ctx, RWContex
>
> ui(1, colour_description);
> if (current->colour_description) {
> -ui(8, colour_primaries);
> -ui(8, transfer_characteristics);
> -ui(8, matrix_coefficients);
> +uir(8, colour_primaries, 1, 255);
> +uir(8, transfer_characteristics, 1, 255);
> +uir(8, matrix_coefficients, 1, 255);
> }
>
> ui(14, display_horizontal_size);
> @@ -366,16 +366,16 @@ static int FUNC(slice_header)(CodedBitstreamContext
> *ctx, RWContext *rw,
> if (!current->extra_information)
> return AVERROR(ENOMEM);
> for (k = 0; k < current->extra_information_length; k++) {
> -xui(1, extra_bit_slice, bit, 0);
> +xui(1, extra_bit_slice, bit, 1, 1, 0);
> xui(8, extra_information_slice[k],
> -current->extra_information[k], 1, k);
> +current->extra_information[k], 0, 255, 1, k);
> }
> }
> #else
> for (k = 0; k < current->extra_information_length; k++) {
> -xui(1, extra_bit_slice, 1, 0);
> +xui(1, extra_bit_slice, 1, 1, 1, 0);
> xui(8,
Re: [FFmpeg-devel] [PATCH]lavfi/frei0r: Fix union and remove unneeded cast
On Wed, Apr 24, 2019 at 5:12 AM Carl Eugen Hoyos wrote: > > Hi! > > I failed to test attached patch but it seems like a more useful fix for > the following (past) warning: > libavfilter/vf_frei0r.c:130:17: warning: assignment to ‘char **’ from > incompatible pointer type ‘char *’ > > Please comment, Carl Eugen LGTM, looks like I given a wrong type casting in this case. ___ 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] [FFmpeg-cvslog] lavfi/frei0r: Fixes the compilation warnings
On Wed, Apr 24, 2019 at 5:04 AM Carl Eugen Hoyos wrote: > > 2019-04-21 15:21 GMT+02:00, Jun Zhao : > > ffmpeg | branch: master | Jun Zhao | Sun Apr 21 > > 12:37:29 2019 +0800| [b272d5b9b6e189cb855ad393edf8524066bd0d07] | committer: > > Jun Zhao > > > > lavfi/frei0r: Fixes the compilation warnings > > > > Fixes the compilation warnings > > > > Reviewed-by: Paul B Mahol > > Signed-off-by: Jun Zhao > > > >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=b272d5b9b6e189cb855ad393edf8524066bd0d07 > > --- > > > > libavfilter/vf_frei0r.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c > > index c775ed1d99..165fbd7d81 100644 > > --- a/libavfilter/vf_frei0r.c > > +++ b/libavfilter/vf_frei0r.c > > @@ -127,7 +127,7 @@ static int set_param(AVFilterContext *ctx, > > f0r_param_info_t info, int index, cha > > break; > > > > case F0R_PARAM_STRING: > > -val.str = param; > > +val.str = (f0r_param_string *)param; > > How can I test this? > (Did you test it?) > > Carl Eugen You can build ffmpeg with frei0r in Ubuntu 18.04, if don't have this patch, it's will get the warning like: libavfilter/vf_frei0r.c:131:17: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types] val.str = param; ^ gcc -v get out put as follow: Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/7/lto-wrapper OFFLOAD_TARGET_NAMES=nvptx-none OFFLOAD_TARGET_DEFAULT=1 Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 7.3.0-27ubuntu1~18.04' --with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-7 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 7.3.0 (Ubuntu 7.3.0-27ubuntu1~18.04) ___ 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]tests/run-sh: Add execsuf to some calls of local tools
2019-04-19 18:05 GMT+02:00, Carl Eugen Hoyos : > 2019-04-19 16:12 GMT+02:00, Carl Eugen Hoyos : >> 2019-04-19 13:11 GMT+02:00, Carl Eugen Hoyos : >> >>> Attached patch is needed here for fate on wsl with msvc, not >>> sure why I didn't need this with mingw-gcc. >> >> Better tested and correct variant attached. > > And a third attempt. > No idea why the others initially worked... Patch applied. Carl Eugen ___ 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] [MSVC toolchain] Patch to allow building FFmpeg with Linux bash on Windows (WSL)
2017-12-29 13:16 GMT+01:00, Cyber Sinh : > Sorry for the diff instead of regular git patch. Here is the patch. Only saw this today after wondering why nobody tried using wsl before... How can I test this part of the patch? In which situation is it supposed to make a difference? diff --git a/compat/windows/mslink b/compat/windows/mslink index 07b2b3e378..9b6b83c4ed 100755 --- a/compat/windows/mslink +++ b/compat/windows/mslink @@ -1,9 +1,9 @@ #!/bin/sh -LINK_EXE_PATH=$(dirname "$(command -v cl)")/link +LINK_EXE_PATH=$(dirname "$(command -v cl.exe)")/link.exe if [ -x "$LINK_EXE_PATH" ]; then "$LINK_EXE_PATH" $@ else I will apply the makedef part that I just tested successfully. Carl Eugen > > > -Message d'origine- > De : ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] De la part de > wm4 > Envoyé : vendredi 29 décembre 2017 12:54 > À : ffmpeg-devel@ffmpeg.org > Objet : Re: [FFmpeg-devel] [MSVC toolchain] Patch to allow building FFmpeg > with Linux bash on Windows (WSL) > > On Fri, 29 Dec 2017 10:44:21 +0100 > Hendrik Leppkes wrote: > >> On Fri, Dec 29, 2017 at 3:43 AM, Cyber Sinh wrote: >> > The attached patch changes the configure script for FFmpeg (and >> > associated shell scripts) to call MSVC tools including their >> > extensions (cl.exe instead of cl for example). This is necessary, >> > because WSL can automatically launch Windows processes from the >> > Linux side but only if the process is in the path and includes the >> > full name. Linux doesn't automatically append .exe and such to invoke a >> > file. >> > >> > >> >> The attached file is corrupted. Please ensure its in git format-patch >> format. As it is, I can't even read it at all. > > It's in UTF-16, and just a diff. Yeah, nobody can comfortably read it. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ 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 14/15] avformat/matroskaenc: Improve log messages for blocks
On Mon, Apr 22, 2019 at 11:14:00PM +, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Sun, Apr 21, 2019 at 11:04:00PM +, Andreas Rheinhardt wrote: > >> Michael Niedermayer: > >>> On Sat, Apr 20, 2019 at 01:41:09AM +0200, Andreas Rheinhardt wrote: > Up until now, a block's relative offset has been reported as the offset > in the log messages output when writing blocks; given that it is > impossible to know the real offset from the beginning of the file at > this point due to the fact that it is not yet known how many bytes will > be used for the containing cluster's length field both the relative > offset in the cluster as well as the offset of the containing cluster > will be reported from now on. > > Also, the log message for writing vtt blocks has been brought in line > with the message for normal blocks. > > Signed-off-by: Andreas Rheinhardt > --- > libavformat/matroskaenc.c | 15 --- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index 441315e2d5..fc0e95440e 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -2124,10 +2124,10 @@ static void mkv_write_block(AVFormatContext *s, > AVIOContext *pb, > > ts += mkv->tracks[pkt->stream_index].ts_offset; > > -av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size > %d, " > - "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", > keyframe %d\n", > - avio_tell(pb), pkt->size, pkt->pts, pkt->dts, pkt->duration, > - keyframe != 0); > +av_log(s, AV_LOG_DEBUG, "Writing block at relative offset %" PRId64 > " in " > + "cluster at offset %" PRId64 "; size %d, pts %" PRId64 ", > dts %" PRId64 > + ", duration %" PRId64 ", keyframe %d\n", avio_tell(pb), > mkv->cluster_pos, > + pkt->size, pkt->pts, pkt->dts, pkt->duration, keyframe != 0); > if (par->codec_id == AV_CODEC_ID_H264 && par->extradata_size > 0 && > (AV_RB24(par->extradata) == 1 || AV_RB32(par->extradata) == 1)) > ff_avc_parse_nal_units_buf(pkt->data, , ); > >>> > @@ -2231,9 +2231,10 @@ static int mkv_write_vtt_blocks(AVFormatContext > *s, AVIOContext *pb, AVPacket *p > > size = id_size + 1 + settings_size + 1 + pkt->size; > > -av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size > %d, " > - "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", > flags %d\n", > - avio_tell(pb), size, pkt->pts, pkt->dts, pkt->duration, > flags); > +av_log(s, AV_LOG_DEBUG, "Writing block at relative offset %" PRId64 > " in " > + "cluster at offset %" PRId64 "; size %d, pts %" PRId64 ", > dts %" PRId64 > + ", duration %" PRId64 ", keyframe %d\n", avio_tell(pb), > mkv->cluster_pos, > + size, pkt->pts, pkt->dts, pkt->duration, 1); > >>> > >>> It does feel a bit odd to pass a litteral 1 as argument to %d to print a > >>> 1. > >>> why is that not a "1" or ommited ? > >>> also these 2 look a bit duplicated > >>> and its a unreadable spagethi, iam sure this can be made more readable > >>> with > >>> some line break changes > >> > >> The duplication is intentional: It means that there needs to be only > >> one string literal in the actual binary. Given that the first > > > > hmm, ok > > it is still duplicated in the source though and this can mutate by being > > edited by someone not knowing it should stay in sync. > > so it may be better to put that string in s static const or #define > > or the printing code itself could be in its own function > > > Actually, I intend to integrate the writing of WebVTT subtitles into > the main write_block function. This will probably be done once I find > the time to implement support for the Matroska way of WebVTT subtitles > (currently only the WebM way is supported). This uses BlockAdditions > and so it would be natural to do this inside write_block as it already > contains code for writing block additions. > But for the time being, a comment will suffice. (The last change to > this part of the code happened four years ago, so I don't think that > someone will inadvertently unsync them any time soon.) ok thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope 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] libavformat: fix inputs initialization in mpegts muxer with filters
On Tue, Apr 23, 2019 at 11:55:59PM +0200, Michael Niedermayer wrote:
> On Tue, Apr 23, 2019 at 07:16:07AM +, Andreas Håkon wrote:
> >
> > ‐‐‐ Original Message ‐‐‐
> > On Tuesday, 23 de April de 2019 1:07, Michael Niedermayer
> > wrote:
> >
> > > On Mon, Apr 22, 2019 at 11:42:57AM +, Andreas Håkon wrote:
> > >
> > >
> > > > Please, revise the code! I hope you understand it when you look at my
> > > > descriptions.
> > >
> > > I would prefer to have some testcase that shows a problem
> > > what you write basically sounds like
> > > there are 2 similar pieces of code but they differ, so you change
> > > one assuming the other is correct. Maybe the change is correct but
> > > the explanation is insufficient
> > >
> >
> > Hi Michael,
> >
> > I agree that a testcase is preferable. However, I can use the Mathematical
> > Demonstration
> > by reduction to the absurd:
> >
> > - The only difference between block A and B (the two "if () ... else if ()
> > ...") is that the
> > first one is executed when a filter graph is running.
> > - In the second block (block B) the "ost->inputs_done = 1;" is executed
> > prior to return.
> > - In the first block (bloc A) is not executed.
> >
> > Why is then executed in the Block B?
> >
> > If the only difference is the filter graph, and nothing related to inputs,
> > then only one
> > of these two can be true:
> >
> > 1) Or the Block B doesn't needs the code "ost->inputs_done = 1;"
> > 2) Or the Block A needs it too.
> >
> > And I don't think the option 1 is true.
> >
> > So by reduction to the absurd is demonstrated.
> > Regards.
>
> nothing of what you write here has any resemblance of a Mathematical proof,
> not that it matters but you claimed that.
> A mathematical proof requires clear definitions and also requires the
> proposition
> of what one proofs to be well existing at least.
>
> I think a discussion of mathematical proofs will not help this matter
>
> I think a more accurate description of the situation is
> 1. we both see that there are 2 pieces of code that differ in a way that looks
>odd
> 2. we both do not know why.
> 3. you are convinced that remooving this difference in some random way that
>looks sensible is a good idea while i think its not a good idea to make
>such change without better understanding first.
some analysis of the change of this patch:
The code part that is changed in the patch first checks
if (!ost->initialized) {
and if true runs
ret = init_output_stream(ost, error, sizeof(error));
if (ret < 0) {
av_log(NULL, AV_LOG_ERROR, "Error initializing output stream
%d:%d -- %s\n",
ost->file_index, ost->index, error);
exit_program(1);
init_output_stream() on failure exits the program, on success
it does
ost->initialized = 1;
Thus we know ost->initialized must be non zero after this
and there is no code that resets it to 0 that i can see
and the only use (read) case of ost->inputs_done is
if (!ost->initialized && !ost->inputs_done)
so the code you want to add is dead code and will have no effect.
Thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
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".
[FFmpeg-devel] [PATCH v3 2/4] cbs_mpeg2: Improve checks for invalid values
horizontal/vertical_size_value (containing the twelve least significant
bits of the frame size) mustn't be zero according to the specifications;
and the value 0 is forbidden for the colour_description elements.
Signed-off-by: Andreas Rheinhardt
---
The actual function calls after macro expansion are the same as in the
earlier versions with the exception of the extra_bit_slice calls.
libavcodec/cbs_mpeg2.c | 14 --
libavcodec/cbs_mpeg2_syntax_template.c | 20 ++--
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index cdde68ea38..fc21745a51 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -41,20 +41,22 @@
#define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs, __VA_ARGS__
}) : NULL)
#define ui(width, name) \
-xui(width, name, current->name, 0)
+xui(width, name, current->name, 0, MAX_UINT_BITS(width), 0)
+#define uir(width, name, range_min, range_max) \
+xui(width, name, current->name, range_min, range_max, 0)
#define uis(width, name, subs, ...) \
-xui(width, name, current->name, subs, __VA_ARGS__)
+xui(width, name, current->name, 0, MAX_UINT_BITS(width), subs,
__VA_ARGS__)
#define READ
#define READWRITE read
#define RWContext GetBitContext
-#define xui(width, name, var, subs, ...) do { \
+#define xui(width, name, var, range_min, range_max, subs, ...) do { \
uint32_t value = 0; \
CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \
SUBSCRIPTS(subs, __VA_ARGS__), \
- , 0, (1 << width) - 1)); \
+ , range_min, range_max)); \
var = value; \
} while (0)
@@ -81,10 +83,10 @@
#define READWRITE write
#define RWContext PutBitContext
-#define xui(width, name, var, subs, ...) do { \
+#define xui(width, name, var, range_min, range_max, subs, ...) do { \
CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \
SUBSCRIPTS(subs, __VA_ARGS__), \
-var, 0, (1 << width) - 1)); \
+var, range_min, range_max)); \
} while (0)
#define marker_bit() do { \
diff --git a/libavcodec/cbs_mpeg2_syntax_template.c
b/libavcodec/cbs_mpeg2_syntax_template.c
index 10aaea7734..27dcaad316 100644
--- a/libavcodec/cbs_mpeg2_syntax_template.c
+++ b/libavcodec/cbs_mpeg2_syntax_template.c
@@ -26,8 +26,8 @@ static int FUNC(sequence_header)(CodedBitstreamContext *ctx,
RWContext *rw,
ui(8, sequence_header_code);
-ui(12, horizontal_size_value);
-ui(12, vertical_size_value);
+uir(12, horizontal_size_value, 1, 4095);
+uir(12, vertical_size_value, 1, 4095);
mpeg2->horizontal_size = current->horizontal_size_value;
mpeg2->vertical_size = current->vertical_size_value;
@@ -79,7 +79,7 @@ static int FUNC(user_data)(CodedBitstreamContext *ctx,
RWContext *rw,
#endif
for (k = 0; k < current->user_data_length; k++)
-xui(8, user_data, current->user_data[k], 0);
+xui(8, user_data, current->user_data[k], 0, 255, 0);
return 0;
}
@@ -125,9 +125,9 @@ static int
FUNC(sequence_display_extension)(CodedBitstreamContext *ctx, RWContex
ui(1, colour_description);
if (current->colour_description) {
-ui(8, colour_primaries);
-ui(8, transfer_characteristics);
-ui(8, matrix_coefficients);
+uir(8, colour_primaries, 1, 255);
+uir(8, transfer_characteristics, 1, 255);
+uir(8, matrix_coefficients, 1, 255);
}
ui(14, display_horizontal_size);
@@ -366,16 +366,16 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx,
RWContext *rw,
if (!current->extra_information)
return AVERROR(ENOMEM);
for (k = 0; k < current->extra_information_length; k++) {
-xui(1, extra_bit_slice, bit, 0);
+xui(1, extra_bit_slice, bit, 1, 1, 0);
xui(8, extra_information_slice[k],
-current->extra_information[k], 1, k);
+current->extra_information[k], 0, 255, 1, k);
}
}
#else
for (k = 0; k < current->extra_information_length; k++) {
-xui(1, extra_bit_slice, 1, 0);
+xui(1, extra_bit_slice, 1, 1, 1, 0);
xui(8, extra_information_slice[k],
-current->extra_information[k], 1, k);
+current->extra_information[k], 0, 255, 1, k);
}
#endif
}
--
2.21.0
___
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] lavc/vaapi_encode_h264: add support for maxframesize
On Tue, Apr 23, 2019 at 11:29:17AM +0800, Linjie Fu wrote:
> Add support for max frame size:
> - max_frame_size (bytes) to indicate the allowed max frame size.
> - pass_num to indicate number of passes.
> - delta_qp to indicate adjust qp value.
>
> Currently only AVC encoder can support this settings in multiple pass case.
> If the frame size exceeds, the encoder will do more pak passes to adjust the
> QP value to control the frame size.
>
> Set Default num_passes to 4 (1~4), set delta_qp[4] = {1, 1, 1, 1}, use
> new_qp for encoder if frame size exceeds the limitation:
> new_qp = base_qp + delta_qp[0] + delta_qp[1] + ...
>
> ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo \
> -v verbose -s:v 352x288 -i ./input.yuv -vf format=nv12,hwupload \
> -c:v h264_vaapi -profile:v main -g 30 -bf 3 -max_frame_size 4 \
> -pass_num 2 -delta_qp 2 -vframes 100 -y ./max_frame_size.h264
>
> Signed-off-by: Linjie Fu
> ---
> libavcodec/vaapi_encode.c | 46 ++
> libavcodec/vaapi_encode.h | 11
> libavcodec/vaapi_encode_h264.c | 15 +++
> 3 files changed, 72 insertions(+)
fails to build
CC libavcodec/vaapi_encode.o
In file included from libavcodec/vaapi_encode.c:27:0:
libavcodec/vaapi_encode.h:280:9: error: unknown type name
‘VAEncMiscParameterBufferMultiPassFrameSize’
VAEncMiscParameterBufferMultiPassFrameSize mfs;
^
libavcodec/vaapi_encode.c: In function ‘vaapi_encode_init_max_frame_size’:
libavcodec/vaapi_encode.c:1660:33: error:
‘VAEncMiscParameterTypeMultiPassFrameSize’ undeclared (first use in this
function)
ctx->mfs_params.misc.type = VAEncMiscParameterTypeMultiPassFrameSize;
^
libavcodec/vaapi_encode.c:1660:33: note: each undeclared identifier is reported
only once for each function it appears in
libavcodec/vaapi_encode.c:1661:24: error: request for member ‘type’ in
something not a structure or union
ctx->mfs_params.mfs.type = VAEncMiscParameterTypeMultiPassFrameSize;
^
libavcodec/vaapi_encode.c:1662:24: error: request for member ‘max_frame_size’
in something not a structure or union
ctx->mfs_params.mfs.max_frame_size = max_frame_size;
^
libavcodec/vaapi_encode.c:1663:24: error: request for member ‘num_passes’ in
something not a structure or union
ctx->mfs_params.mfs.num_passes = num_passes;
^
libavcodec/vaapi_encode.c:1664:24: error: request for member ‘delta_qp’ in
something not a structure or union
ctx->mfs_params.mfs.delta_qp = delta_qp;
^
make: *** [libavcodec/vaapi_encode.o] Error 1
CC libavcodec/vaapi_encode_h264.o
In file included from libavcodec/vaapi_encode_h264.c:36:0:
libavcodec/vaapi_encode.h:280:9: error: unknown type name
‘VAEncMiscParameterBufferMultiPassFrameSize’
VAEncMiscParameterBufferMultiPassFrameSize mfs;
^
make: *** [libavcodec/vaapi_encode_h264.o] Error 1
CC libavcodec/vaapi_encode_mpeg2.o
In file included from libavcodec/vaapi_encode_mpeg2.c:28:0:
libavcodec/vaapi_encode.h:280:9: error: unknown type name
‘VAEncMiscParameterBufferMultiPassFrameSize’
VAEncMiscParameterBufferMultiPassFrameSize mfs;
^
make: *** [libavcodec/vaapi_encode_mpeg2.o] Error 1
make: Target `all' not remade because of errors.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
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] avdevice/alsa: fix indefinite stop on closing PCM capture
On Mon, Apr 22, 2019, at 10:47 AM, Nicolas George wrote: > > Sorry, missed it. LGTM. Pushed. f9a061a31c3d2d81b3ec1e1b9b37187a358cdd9e ___ 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/mpegenc - reject unsupported audio streams
2019-04-22 13:00 GMT+02:00, Gyan : > > On 22-04-2019 01:15 PM, Gyan wrote: >> >> >> On 22-04-2019 12:30 PM, Carl Eugen Hoyos wrote: >>> Am 20.04.2019 um 11:31 schrieb Gyan : Old patch that was never applied. Rebased. >>> Please return patch_welcome for mlp and truehd. >> Will do. >>> Wasn’t there another comment (not by me): “Why >>> can’t .codec_tag be used?” >> >> There's no codec_tag member set. This patch is just >> to disallow an invalid muxing to proceed. Wasn't the argument that this is possible with codec_tag? > Revised. What about aac? And can't you wait more than a few hours for a review? Carl Eugen ___ 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] libavformat: fix inputs initialization in mpegts muxer with filters
On Tue, Apr 23, 2019 at 07:16:07AM +, Andreas Håkon wrote: > > ‐‐‐ Original Message ‐‐‐ > On Tuesday, 23 de April de 2019 1:07, Michael Niedermayer > wrote: > > > On Mon, Apr 22, 2019 at 11:42:57AM +, Andreas Håkon wrote: > > > > > > > Please, revise the code! I hope you understand it when you look at my > > > descriptions. > > > > I would prefer to have some testcase that shows a problem > > what you write basically sounds like > > there are 2 similar pieces of code but they differ, so you change > > one assuming the other is correct. Maybe the change is correct but > > the explanation is insufficient > > > > Hi Michael, > > I agree that a testcase is preferable. However, I can use the Mathematical > Demonstration > by reduction to the absurd: > > - The only difference between block A and B (the two "if () ... else if () > ...") is that the > first one is executed when a filter graph is running. > - In the second block (block B) the "ost->inputs_done = 1;" is executed prior > to return. > - In the first block (bloc A) is not executed. > > Why is then executed in the Block B? > > If the only difference is the filter graph, and nothing related to inputs, > then only one > of these two can be true: > > 1) Or the Block B doesn't needs the code "ost->inputs_done = 1;" > 2) Or the Block A needs it too. > > And I don't think the option 1 is true. > > So by reduction to the absurd is demonstrated. > Regards. nothing of what you write here has any resemblance of a Mathematical proof, not that it matters but you claimed that. A mathematical proof requires clear definitions and also requires the proposition of what one proofs to be well existing at least. I think a discussion of mathematical proofs will not help this matter I think a more accurate description of the situation is 1. we both see that there are 2 pieces of code that differ in a way that looks odd 2. we both do not know why. 3. you are convinced that remooving this difference in some random way that looks sensible is a good idea while i think its not a good idea to make such change without better understanding first. thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Take away the freedom of one citizen and you will be jailed, take away the freedom of all citizens and you will be congratulated by your peers in Parliament. 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".
[FFmpeg-devel] [PATCH]lavfi/frei0r: Fix union and remove unneeded cast
Hi! I failed to test attached patch but it seems like a more useful fix for the following (past) warning: libavfilter/vf_frei0r.c:130:17: warning: assignment to ‘char **’ from incompatible pointer type ‘char *’ Please comment, Carl Eugen From c0d6001012f3da95ee493905c7411dc20839f4c8 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos Date: Tue, 23 Apr 2019 23:09:01 +0200 Subject: [PATCH] lavfi/frei0r: Fix a union entry and remove an unneeded cast. --- libavfilter/vf_frei0r.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c index 165fbd7..5d38405 100644 --- a/libavfilter/vf_frei0r.c +++ b/libavfilter/vf_frei0r.c @@ -93,7 +93,7 @@ static int set_param(AVFilterContext *ctx, f0r_param_info_t info, int index, cha double d; f0r_param_color_t col; f0r_param_position_t pos; -f0r_param_string *str; +f0r_param_string str; } val; char *tail; uint8_t rgba[4]; @@ -127,7 +127,7 @@ static int set_param(AVFilterContext *ctx, f0r_param_info_t info, int index, cha break; case F0R_PARAM_STRING: -val.str = (f0r_param_string *)param; +val.str = param; break; } -- 1.7.10.4 ___ 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] [FFmpeg-cvslog] lavfi/frei0r: Fixes the compilation warnings
2019-04-21 15:21 GMT+02:00, Jun Zhao : > ffmpeg | branch: master | Jun Zhao | Sun Apr 21 > 12:37:29 2019 +0800| [b272d5b9b6e189cb855ad393edf8524066bd0d07] | committer: > Jun Zhao > > lavfi/frei0r: Fixes the compilation warnings > > Fixes the compilation warnings > > Reviewed-by: Paul B Mahol > Signed-off-by: Jun Zhao > >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=b272d5b9b6e189cb855ad393edf8524066bd0d07 > --- > > libavfilter/vf_frei0r.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c > index c775ed1d99..165fbd7d81 100644 > --- a/libavfilter/vf_frei0r.c > +++ b/libavfilter/vf_frei0r.c > @@ -127,7 +127,7 @@ static int set_param(AVFilterContext *ctx, > f0r_param_info_t info, int index, cha > break; > > case F0R_PARAM_STRING: > -val.str = param; > +val.str = (f0r_param_string *)param; How can I test this? (Did you test it?) Carl Eugen ___ 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 2/4] cbs_mpeg2: Improve checks for invalid values
On 4/23/2019 5:32 PM, Andreas Rheinhardt wrote:
> horizontal/vertical_size_value (containing the twelve least significant
> bits of the frame size) mustn't be zero according to the specifications;
> and the value 0 is forbidden for the colour_description elements.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> Given that xui is directly used in the syntax template, I had the
> options a) to add xuir (as I did in my earlier attempt),
> b) to change xui to accept ranges and change the syntax template (add
> the ranges equaling "no custom range") or
> c) change xui and add a new macro for what xui currently does.
> I opted for c). The new macro is named xuib, b in analogy to your
> proposal for cbs_h2645. Unfortunately, the naming of ui is inconsistent
> with this, but changing this would basically touch every line in the
> syntax template.
I think b) is the best option, since my idea was precisely to not add a
new x* macro that duplicates or wraps another x* macro.
If you look at cbs_h2645, the xu() macro is also used directly in the
syntax template, and called with the default range equaling "no custom
range" for the requested width.
Also, unrelated to this patch, but when you send updated patch sets
consider starting a new thread for them rather than as a reply to the
previous one, and adding a v2, v3, etc, depending on what iteration
we're talking about, to each email's subject after the patch number.
That way is easier to locate and also doesn't get mixed with previous
sets and their review replies.
> libavcodec/cbs_mpeg2.c | 17 ++---
> libavcodec/cbs_mpeg2_syntax_template.c | 24
> 2 files changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index cdde68ea38..4025812531 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -41,20 +41,23 @@
> #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs,
> __VA_ARGS__ }) : NULL)
>
> #define ui(width, name) \
> -xui(width, name, current->name, 0)
> +xuib(width, name, current->name, 0)
> +#define uir(width, name, range_min, range_max) \
> +xui(width, name, current->name, range_min, range_max, 0)
> #define uis(width, name, subs, ...) \
> -xui(width, name, current->name, subs, __VA_ARGS__)
> -
> +xuib(width, name, current->name, subs, __VA_ARGS__)
> +#define xuib(width, name, var, subs, ...) \
> +xui(width, name, var, 0, MAX_UINT_BITS(width), subs, __VA_ARGS__)
>
> #define READ
> #define READWRITE read
> #define RWContext GetBitContext
>
> -#define xui(width, name, var, subs, ...) do { \
> +#define xui(width, name, var, range_min, range_max, subs, ...) do { \
> uint32_t value = 0; \
> CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \
> SUBSCRIPTS(subs, __VA_ARGS__), \
> - , 0, (1 << width) - 1)); \
> + , range_min, range_max)); \
> var = value; \
> } while (0)
>
> @@ -81,10 +84,10 @@
> #define READWRITE write
> #define RWContext PutBitContext
>
> -#define xui(width, name, var, subs, ...) do { \
> +#define xui(width, name, var, range_min, range_max, subs, ...) do { \
> CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \
> SUBSCRIPTS(subs, __VA_ARGS__), \
> -var, 0, (1 << width) - 1)); \
> +var, range_min, range_max)); \
> } while (0)
>
> #define marker_bit() do { \
> diff --git a/libavcodec/cbs_mpeg2_syntax_template.c
> b/libavcodec/cbs_mpeg2_syntax_template.c
> index 10aaea7734..73bfeae29e 100644
> --- a/libavcodec/cbs_mpeg2_syntax_template.c
> +++ b/libavcodec/cbs_mpeg2_syntax_template.c
> @@ -26,8 +26,8 @@ static int FUNC(sequence_header)(CodedBitstreamContext
> *ctx, RWContext *rw,
>
> ui(8, sequence_header_code);
>
> -ui(12, horizontal_size_value);
> -ui(12, vertical_size_value);
> +uir(12, horizontal_size_value, 1, 4095);
> +uir(12, vertical_size_value, 1, 4095);
>
> mpeg2->horizontal_size = current->horizontal_size_value;
> mpeg2->vertical_size = current->vertical_size_value;
> @@ -79,7 +79,7 @@ static int FUNC(user_data)(CodedBitstreamContext *ctx,
> RWContext *rw,
> #endif
>
> for (k = 0; k < current->user_data_length; k++)
> -xui(8, user_data, current->user_data[k], 0);
> +xuib(8, user_data, current->user_data[k], 0);
>
> return 0;
> }
> @@ -125,9 +125,9 @@ static int
> FUNC(sequence_display_extension)(CodedBitstreamContext *ctx, RWContex
>
> ui(1, colour_description);
> if (current->colour_description) {
> -ui(8, colour_primaries);
> -ui(8, transfer_characteristics);
> -ui(8, matrix_coefficients);
> +uir(8, colour_primaries, 1, 255);
> +uir(8,
[FFmpeg-devel] [PATCH 3/4] mpeg2_metadata, cbs_mpeg2: Fix handling of colour_description
If a sequence display extension is read with colour_description equal to
zero, but a user wants to add one or more of the colour_description
elements, then the colour_description elements the user did not explicitly
request to be set are set to zero and not to the value equal to
unknown/unspecified (namely 2). A value of zero is not only inappropriate,
but explicitly forbidden. This is fixed by inferring the right default
values during the reading process if the elements are absent; moreover,
changing any of the colour_description elements to zero is now no longer
permitted.
Furthermore, if a sequence display extension has to be added, the
earlier code set some fields to their default value twice. This has been
changed, too.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cbs_mpeg2.c | 15 +++
libavcodec/cbs_mpeg2_syntax_template.c | 4
libavcodec/mpeg2_metadata_bsf.c| 18 --
3 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 4025812531..96c2d2c22e 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -70,6 +70,10 @@
(get_bits_left(rw) >= width && \
(var = show_bits(rw, width)) == (compare))
+#define infer(name, value) do { \
+current->name = value; \
+} while (0)
+
#include "cbs_mpeg2_syntax_template.c"
#undef READ
@@ -78,6 +82,7 @@
#undef xui
#undef marker_bit
#undef nextbits
+#undef infer
#define WRITE
@@ -96,6 +101,15 @@
#define nextbits(width, compare, var) (var)
+#define infer(name, value) do { \
+if (current->name != (value)) { \
+av_log(ctx->log_ctx, AV_LOG_WARNING, "Warning: " \
+ "%s does not match inferred value: " \
+ "%"PRId64", but should be %"PRId64".\n", \
+ #name, (int64_t)current->name, (int64_t)(value)); \
+} \
+} while (0)
+
#include "cbs_mpeg2_syntax_template.c"
#undef READ
@@ -104,6 +118,7 @@
#undef xui
#undef marker_bit
#undef nextbits
+#undef infer
static void cbs_mpeg2_free_user_data(void *unit, uint8_t *content)
diff --git a/libavcodec/cbs_mpeg2_syntax_template.c
b/libavcodec/cbs_mpeg2_syntax_template.c
index 73bfeae29e..f2718635c2 100644
--- a/libavcodec/cbs_mpeg2_syntax_template.c
+++ b/libavcodec/cbs_mpeg2_syntax_template.c
@@ -128,6 +128,10 @@ static int
FUNC(sequence_display_extension)(CodedBitstreamContext *ctx, RWContex
uir(8, colour_primaries, 1, 255);
uir(8, transfer_characteristics, 1, 255);
uir(8, matrix_coefficients, 1, 255);
+} else {
+infer(colour_primaries, 2);
+infer(transfer_characteristics, 2);
+infer(matrix_coefficients, 2);
}
ui(14, display_horizontal_size);
diff --git a/libavcodec/mpeg2_metadata_bsf.c b/libavcodec/mpeg2_metadata_bsf.c
index ba3a74afda..292c9a17ce 100644
--- a/libavcodec/mpeg2_metadata_bsf.c
+++ b/libavcodec/mpeg2_metadata_bsf.c
@@ -147,18 +147,12 @@ static int mpeg2_metadata_update_fragment(AVBSFContext
*bsf,
if (ctx->colour_primaries >= 0)
sde->colour_primaries = ctx->colour_primaries;
-else if (add_sde)
-sde->colour_primaries = 2;
if (ctx->transfer_characteristics >= 0)
sde->transfer_characteristics = ctx->transfer_characteristics;
-else if (add_sde)
-sde->transfer_characteristics = 2;
if (ctx->matrix_coefficients >= 0)
sde->matrix_coefficients = ctx->matrix_coefficients;
-else if (add_sde)
-sde->matrix_coefficients = 2;
}
}
@@ -229,6 +223,18 @@ static int mpeg2_metadata_init(AVBSFContext *bsf)
CodedBitstreamFragment *frag = >fragment;
int err;
+#define VALIDITY_CHECK(name) do { \
+if (!ctx->name) { \
+av_log(bsf, AV_LOG_ERROR, "The value 0 for %s is " \
+ "forbidden.\n", #name); \
+return AVERROR(EINVAL); \
+} \
+} while (0)
+VALIDITY_CHECK(colour_primaries);
+VALIDITY_CHECK(transfer_characteristics);
+VALIDITY_CHECK(matrix_coefficients);
+#undef VALIDITY_CHECK
+
err = ff_cbs_init(>cbc, AV_CODEC_ID_MPEG2VIDEO, bsf);
if (err < 0)
return err;
--
2.21.0
___
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 4/4] cbs_mpeg2: Correct error codes
Up until now, things that are merely unsupported by cbs_mpeg2 have been declared to be invalid input. This has been changed. Signed-off-by: Andreas Rheinhardt --- libavcodec/cbs_mpeg2.c | 4 +--- libavcodec/cbs_mpeg2_syntax_template.c | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c index 96c2d2c22e..fce881d1c3 100644 --- a/libavcodec/cbs_mpeg2.c +++ b/libavcodec/cbs_mpeg2.c @@ -245,9 +245,7 @@ static int cbs_mpeg2_read_unit(CodedBitstreamContext *ctx, group_of_pictures_header, NULL); #undef START default: -av_log(ctx->log_ctx, AV_LOG_ERROR, "Unknown start code %02"PRIx32".\n", - unit->type); -return AVERROR_INVALIDDATA; +return AVERROR(ENOSYS); } } diff --git a/libavcodec/cbs_mpeg2_syntax_template.c b/libavcodec/cbs_mpeg2_syntax_template.c index f2718635c2..9aa8a378c9 100644 --- a/libavcodec/cbs_mpeg2_syntax_template.c +++ b/libavcodec/cbs_mpeg2_syntax_template.c @@ -323,9 +323,9 @@ static int FUNC(extension_data)(CodedBitstreamContext *ctx, RWContext *rw, return FUNC(picture_coding_extension) (ctx, rw, >data.picture_coding); default: -av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid extension ID %d.\n", +av_log(ctx->log_ctx, AV_LOG_ERROR, "Extension ID %d not supported.\n", current->extension_start_code_identifier); -return AVERROR_INVALIDDATA; +return AVERROR_PATCHWELCOME; } } -- 2.21.0 ___ 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/4] cbs_mpeg2: Correct and use enum values
On 4/23/2019 5:32 PM, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/cbs_mpeg2.c | 30 +++---
> libavcodec/cbs_mpeg2.h | 2 +-
> libavcodec/cbs_mpeg2_syntax_template.c | 10 -
> 3 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index 8b8b266563..cdde68ea38 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -215,13 +215,16 @@ static int cbs_mpeg2_read_unit(CodedBitstreamContext
> *ctx,
> return err; \
> } \
> break;
> -START(0x00, MPEG2RawPictureHeader, picture_header, NULL);
> -START(0xb2, MPEG2RawUserData, user_data,
> -_mpeg2_free_user_data);
> -START(0xb3, MPEG2RawSequenceHeader, sequence_header, NULL);
> -START(0xb5, MPEG2RawExtensionData, extension_data, NULL);
> -START(0xb8, MPEG2RawGroupOfPicturesHeader,
> - group_of_pictures_header, NULL);
> +START(MPEG2_START_PICTURE, MPEG2RawPictureHeader,
> +picture_header,
> NULL);
> +START(MPEG2_START_USER_DATA, MPEG2RawUserData,
> + user_data,
> _mpeg2_free_user_data);
> +START(MPEG2_START_SEQUENCE_HEADER, MPEG2RawSequenceHeader,
> +sequence_header,
> NULL);
> +START(MPEG2_START_EXTENSION, MPEG2RawExtensionData,
> +extension_data,
> NULL);
> +START(MPEG2_START_GROUP, MPEG2RawGroupOfPicturesHeader,
> + group_of_pictures_header,
> NULL);
> #undef START
> default:
> av_log(ctx->log_ctx, AV_LOG_ERROR, "Unknown start code
> %02"PRIx32".\n",
> @@ -244,11 +247,12 @@ static int cbs_mpeg2_write_header(CodedBitstreamContext
> *ctx,
> case start_code: \
> err = cbs_mpeg2_write_ ## func(ctx, pbc, unit->content); \
> break;
> -START(0x00, MPEG2RawPictureHeader, picture_header);
> -START(0xb2, MPEG2RawUserData, user_data);
> -START(0xb3, MPEG2RawSequenceHeader, sequence_header);
> -START(0xb5, MPEG2RawExtensionData, extension_data);
> -START(0xb8, MPEG2RawGroupOfPicturesHeader, group_of_pictures_header);
> +START(MPEG2_START_PICTURE, MPEG2RawPictureHeader,
> picture_header);
> +START(MPEG2_START_USER_DATA, MPEG2RawUserData,
> user_data);
> +START(MPEG2_START_SEQUENCE_HEADER, MPEG2RawSequenceHeader,
> sequence_header);
> +START(MPEG2_START_EXTENSION, MPEG2RawExtensionData,
> extension_data);
> +START(MPEG2_START_GROUP, MPEG2RawGroupOfPicturesHeader,
> +
> group_of_pictures_header);
> #undef START
> default:
> av_log(ctx->log_ctx, AV_LOG_ERROR, "Write unimplemented for start "
> @@ -331,7 +335,7 @@ static int cbs_mpeg2_write_unit(CodedBitstreamContext
> *ctx,
>
> init_put_bits(, priv->write_buffer, priv->write_buffer_size);
>
> -if (unit->type >= 0x01 && unit->type <= 0xaf)
> +if (MPEG2_START_IS_SLICE(unit->type))
> err = cbs_mpeg2_write_slice(ctx, unit, );
> else
> err = cbs_mpeg2_write_header(ctx, unit, );
> diff --git a/libavcodec/cbs_mpeg2.h b/libavcodec/cbs_mpeg2.h
> index 92caa99dc1..7565695acb 100644
> --- a/libavcodec/cbs_mpeg2.h
> +++ b/libavcodec/cbs_mpeg2.h
> @@ -51,7 +51,7 @@ enum {
> MPEG2_EXTENSION_PICTURE_CODING= 0x8,
> MPEG2_EXTENSION_PICTURE_SPATIAL_SCALABLE = 0x9,
> MPEG2_EXTENSION_PICTURE_TEMPORAL_SCALABLE = 0xa,
> -MPEG2_EXTENSION_CAMAERA_PARAMETERS= 0xb,
> +MPEG2_EXTENSION_CAMERA_PARAMETERS = 0xb,
> MPEG2_EXTENSION_ITU_T = 0xc,
> };
>
> diff --git a/libavcodec/cbs_mpeg2_syntax_template.c
> b/libavcodec/cbs_mpeg2_syntax_template.c
> index 88cf453b17..10aaea7734 100644
> --- a/libavcodec/cbs_mpeg2_syntax_template.c
> +++ b/libavcodec/cbs_mpeg2_syntax_template.c
> @@ -303,19 +303,19 @@ static int FUNC(extension_data)(CodedBitstreamContext
> *ctx, RWContext *rw,
> ui(4, extension_start_code_identifier);
>
> switch (current->extension_start_code_identifier) {
> -case 1:
> +case MPEG2_EXTENSION_SEQUENCE:
> return FUNC(sequence_extension)
> (ctx, rw, >data.sequence);
> -case 2:
> +case MPEG2_EXTENSION_SEQUENCE_DISPLAY:
> return FUNC(sequence_display_extension)
> (ctx, rw, >data.sequence_display);
> -case 3:
> +case MPEG2_EXTENSION_QUANT_MATRIX:
> return FUNC(quant_matrix_extension)
> (ctx,
[FFmpeg-devel] [PATCH 2/4] cbs_mpeg2: Improve checks for invalid values
horizontal/vertical_size_value (containing the twelve least significant
bits of the frame size) mustn't be zero according to the specifications;
and the value 0 is forbidden for the colour_description elements.
Signed-off-by: Andreas Rheinhardt
---
Given that xui is directly used in the syntax template, I had the
options a) to add xuir (as I did in my earlier attempt),
b) to change xui to accept ranges and change the syntax template (add
the ranges equaling "no custom range") or
c) change xui and add a new macro for what xui currently does.
I opted for c). The new macro is named xuib, b in analogy to your
proposal for cbs_h2645. Unfortunately, the naming of ui is inconsistent
with this, but changing this would basically touch every line in the
syntax template.
libavcodec/cbs_mpeg2.c | 17 ++---
libavcodec/cbs_mpeg2_syntax_template.c | 24
2 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index cdde68ea38..4025812531 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -41,20 +41,23 @@
#define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs, __VA_ARGS__
}) : NULL)
#define ui(width, name) \
-xui(width, name, current->name, 0)
+xuib(width, name, current->name, 0)
+#define uir(width, name, range_min, range_max) \
+xui(width, name, current->name, range_min, range_max, 0)
#define uis(width, name, subs, ...) \
-xui(width, name, current->name, subs, __VA_ARGS__)
-
+xuib(width, name, current->name, subs, __VA_ARGS__)
+#define xuib(width, name, var, subs, ...) \
+xui(width, name, var, 0, MAX_UINT_BITS(width), subs, __VA_ARGS__)
#define READ
#define READWRITE read
#define RWContext GetBitContext
-#define xui(width, name, var, subs, ...) do { \
+#define xui(width, name, var, range_min, range_max, subs, ...) do { \
uint32_t value = 0; \
CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \
SUBSCRIPTS(subs, __VA_ARGS__), \
- , 0, (1 << width) - 1)); \
+ , range_min, range_max)); \
var = value; \
} while (0)
@@ -81,10 +84,10 @@
#define READWRITE write
#define RWContext PutBitContext
-#define xui(width, name, var, subs, ...) do { \
+#define xui(width, name, var, range_min, range_max, subs, ...) do { \
CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \
SUBSCRIPTS(subs, __VA_ARGS__), \
-var, 0, (1 << width) - 1)); \
+var, range_min, range_max)); \
} while (0)
#define marker_bit() do { \
diff --git a/libavcodec/cbs_mpeg2_syntax_template.c
b/libavcodec/cbs_mpeg2_syntax_template.c
index 10aaea7734..73bfeae29e 100644
--- a/libavcodec/cbs_mpeg2_syntax_template.c
+++ b/libavcodec/cbs_mpeg2_syntax_template.c
@@ -26,8 +26,8 @@ static int FUNC(sequence_header)(CodedBitstreamContext *ctx,
RWContext *rw,
ui(8, sequence_header_code);
-ui(12, horizontal_size_value);
-ui(12, vertical_size_value);
+uir(12, horizontal_size_value, 1, 4095);
+uir(12, vertical_size_value, 1, 4095);
mpeg2->horizontal_size = current->horizontal_size_value;
mpeg2->vertical_size = current->vertical_size_value;
@@ -79,7 +79,7 @@ static int FUNC(user_data)(CodedBitstreamContext *ctx,
RWContext *rw,
#endif
for (k = 0; k < current->user_data_length; k++)
-xui(8, user_data, current->user_data[k], 0);
+xuib(8, user_data, current->user_data[k], 0);
return 0;
}
@@ -125,9 +125,9 @@ static int
FUNC(sequence_display_extension)(CodedBitstreamContext *ctx, RWContex
ui(1, colour_description);
if (current->colour_description) {
-ui(8, colour_primaries);
-ui(8, transfer_characteristics);
-ui(8, matrix_coefficients);
+uir(8, colour_primaries, 1, 255);
+uir(8, transfer_characteristics, 1, 255);
+uir(8, matrix_coefficients, 1, 255);
}
ui(14, display_horizontal_size);
@@ -366,16 +366,16 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx,
RWContext *rw,
if (!current->extra_information)
return AVERROR(ENOMEM);
for (k = 0; k < current->extra_information_length; k++) {
-xui(1, extra_bit_slice, bit, 0);
-xui(8, extra_information_slice[k],
-current->extra_information[k], 1, k);
+xuib(1, extra_bit_slice, bit, 0);
+xuib(8, extra_information_slice[k],
+ current->extra_information[k], 1, k);
}
}
#else
for (k = 0; k < current->extra_information_length; k++) {
-xui(1, extra_bit_slice, 1, 0);
-xui(8,
[FFmpeg-devel] [PATCH 1/4] cbs_mpeg2: Correct and use enum values
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cbs_mpeg2.c | 30 +++---
libavcodec/cbs_mpeg2.h | 2 +-
libavcodec/cbs_mpeg2_syntax_template.c | 10 -
3 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 8b8b266563..cdde68ea38 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -215,13 +215,16 @@ static int cbs_mpeg2_read_unit(CodedBitstreamContext *ctx,
return err; \
} \
break;
-START(0x00, MPEG2RawPictureHeader, picture_header, NULL);
-START(0xb2, MPEG2RawUserData, user_data,
-_mpeg2_free_user_data);
-START(0xb3, MPEG2RawSequenceHeader, sequence_header, NULL);
-START(0xb5, MPEG2RawExtensionData, extension_data, NULL);
-START(0xb8, MPEG2RawGroupOfPicturesHeader,
- group_of_pictures_header, NULL);
+START(MPEG2_START_PICTURE, MPEG2RawPictureHeader,
+picture_header, NULL);
+START(MPEG2_START_USER_DATA, MPEG2RawUserData,
+ user_data, _mpeg2_free_user_data);
+START(MPEG2_START_SEQUENCE_HEADER, MPEG2RawSequenceHeader,
+sequence_header, NULL);
+START(MPEG2_START_EXTENSION, MPEG2RawExtensionData,
+extension_data, NULL);
+START(MPEG2_START_GROUP, MPEG2RawGroupOfPicturesHeader,
+ group_of_pictures_header, NULL);
#undef START
default:
av_log(ctx->log_ctx, AV_LOG_ERROR, "Unknown start code
%02"PRIx32".\n",
@@ -244,11 +247,12 @@ static int cbs_mpeg2_write_header(CodedBitstreamContext
*ctx,
case start_code: \
err = cbs_mpeg2_write_ ## func(ctx, pbc, unit->content); \
break;
-START(0x00, MPEG2RawPictureHeader, picture_header);
-START(0xb2, MPEG2RawUserData, user_data);
-START(0xb3, MPEG2RawSequenceHeader, sequence_header);
-START(0xb5, MPEG2RawExtensionData, extension_data);
-START(0xb8, MPEG2RawGroupOfPicturesHeader, group_of_pictures_header);
+START(MPEG2_START_PICTURE, MPEG2RawPictureHeader,
picture_header);
+START(MPEG2_START_USER_DATA, MPEG2RawUserData, user_data);
+START(MPEG2_START_SEQUENCE_HEADER, MPEG2RawSequenceHeader,
sequence_header);
+START(MPEG2_START_EXTENSION, MPEG2RawExtensionData,
extension_data);
+START(MPEG2_START_GROUP, MPEG2RawGroupOfPicturesHeader,
+
group_of_pictures_header);
#undef START
default:
av_log(ctx->log_ctx, AV_LOG_ERROR, "Write unimplemented for start "
@@ -331,7 +335,7 @@ static int cbs_mpeg2_write_unit(CodedBitstreamContext *ctx,
init_put_bits(, priv->write_buffer, priv->write_buffer_size);
-if (unit->type >= 0x01 && unit->type <= 0xaf)
+if (MPEG2_START_IS_SLICE(unit->type))
err = cbs_mpeg2_write_slice(ctx, unit, );
else
err = cbs_mpeg2_write_header(ctx, unit, );
diff --git a/libavcodec/cbs_mpeg2.h b/libavcodec/cbs_mpeg2.h
index 92caa99dc1..7565695acb 100644
--- a/libavcodec/cbs_mpeg2.h
+++ b/libavcodec/cbs_mpeg2.h
@@ -51,7 +51,7 @@ enum {
MPEG2_EXTENSION_PICTURE_CODING= 0x8,
MPEG2_EXTENSION_PICTURE_SPATIAL_SCALABLE = 0x9,
MPEG2_EXTENSION_PICTURE_TEMPORAL_SCALABLE = 0xa,
-MPEG2_EXTENSION_CAMAERA_PARAMETERS= 0xb,
+MPEG2_EXTENSION_CAMERA_PARAMETERS = 0xb,
MPEG2_EXTENSION_ITU_T = 0xc,
};
diff --git a/libavcodec/cbs_mpeg2_syntax_template.c
b/libavcodec/cbs_mpeg2_syntax_template.c
index 88cf453b17..10aaea7734 100644
--- a/libavcodec/cbs_mpeg2_syntax_template.c
+++ b/libavcodec/cbs_mpeg2_syntax_template.c
@@ -303,19 +303,19 @@ static int FUNC(extension_data)(CodedBitstreamContext
*ctx, RWContext *rw,
ui(4, extension_start_code_identifier);
switch (current->extension_start_code_identifier) {
-case 1:
+case MPEG2_EXTENSION_SEQUENCE:
return FUNC(sequence_extension)
(ctx, rw, >data.sequence);
-case 2:
+case MPEG2_EXTENSION_SEQUENCE_DISPLAY:
return FUNC(sequence_display_extension)
(ctx, rw, >data.sequence_display);
-case 3:
+case MPEG2_EXTENSION_QUANT_MATRIX:
return FUNC(quant_matrix_extension)
(ctx, rw, >data.quant_matrix);
-case 7:
+case MPEG2_EXTENSION_PICTURE_DISPLAY:
return FUNC(picture_display_extension)
(ctx, rw, >data.picture_display);
-case 8:
+case MPEG2_EXTENSION_PICTURE_CODING:
return FUNC(picture_coding_extension)
[FFmpeg-devel] [PATCH 14/15] avformat/matroskaenc: Improve log messages for blocks
Up until now, a block's relative offset has been reported as the offset in the log messages output when writing blocks; given that it is impossible to know the real offset from the beginning of the file at this point due to the fact that it is not yet known how many bytes will be used for the containing cluster's length field both the relative offset in the cluster as well as the offset of the containing cluster will be reported from now on. Furthermore, the TrackNumber of the written block has been added to the log output. Also, the log message for writing vtt blocks has been brought in line with the message for normal blocks. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskaenc.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 441315e2d5..01ccc9524c 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -2124,10 +2124,14 @@ static void mkv_write_block(AVFormatContext *s, AVIOContext *pb, ts += mkv->tracks[pkt->stream_index].ts_offset; -av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size %d, " - "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", keyframe %d\n", - avio_tell(pb), pkt->size, pkt->pts, pkt->dts, pkt->duration, - keyframe != 0); +/* The following string is identical to the one in mkv_write_vtt_blocks + * so that only one copy needs to exist in binaries. */ +av_log(s, AV_LOG_DEBUG, + "Writing block of size %d with pts %" PRId64 ", dts %" PRId64 ", " + "duration %" PRId64 " at relative offset %" PRId64 " in cluster " + "at offset %" PRId64 ". TrackNumber %d, keyframe %d\n", + pkt->size, pkt->pts, pkt->dts, pkt->duration, avio_tell(pb), + mkv->cluster_pos, track_number, keyframe != 0); if (par->codec_id == AV_CODEC_ID_H264 && par->extradata_size > 0 && (AV_RB24(par->extradata) == 1 || AV_RB32(par->extradata) == 1)) ff_avc_parse_nal_units_buf(pkt->data, , ); @@ -2231,9 +2235,14 @@ static int mkv_write_vtt_blocks(AVFormatContext *s, AVIOContext *pb, AVPacket *p size = id_size + 1 + settings_size + 1 + pkt->size; -av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size %d, " - "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", flags %d\n", - avio_tell(pb), size, pkt->pts, pkt->dts, pkt->duration, flags); +/* The following string is identical to the one in mkv_write_block so that + * only one copy needs to exist in binaries. */ +av_log(s, AV_LOG_DEBUG, + "Writing block of size %d with pts %" PRId64 ", dts %" PRId64 ", " + "duration %" PRId64 " at relative offset %" PRId64 " in cluster " + "at offset %" PRId64 ". TrackNumber %d, keyframe %d\n", + size, pkt->pts, pkt->dts, pkt->duration, avio_tell(pb), + mkv->cluster_pos, pkt->stream_index + 1, 1); blockgroup = start_ebml_master(pb, MATROSKA_ID_BLOCKGROUP, mkv_blockgroup_size(size)); -- 2.21.0 ___ 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] ffmpeg: Add option to force a specific decode format
2018-11-11 15:54 GMT+01:00, Mark Thompson :
> Fixes #7519.
> +{ "decode_format", OPT_VIDEO | OPT_STRING | HAS_ARG | OPT_EXPERT |
> +OPT_SPEC | OPT_INPUT,
> { .off = OFFSET(decode_formats) },
> +"set output format used by decoder, fail if this format is not
> available", "format" },
Sorry if this was already asked:
Why can't you use the input option "pix_fmt"?
Carl Eugen
___
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] libavformat/mov: limit nb_frames_for_fps to INT_MAX
On Mon, Apr 22, 2019 at 11:05:00AM -0700, Dan Sanders wrote: > It's this or add overflow detection in mov_read_header(). > --- > libavformat/mov.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) will apply thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When the tyrant has disposed of foreign enemies by conquest or treaty, and there is nothing more to fear from them, then he is always stirring up some war or other, in order that the people may require a leader. -- Plato 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 v2] ffmpeg: Add option to force a specific decode format
*bump*
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of Mark
> Thompson
> Sent: Wednesday, February 20, 2019 1:52 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH v2] ffmpeg: Add option to force a specific
> decode format
>
> On 18/02/2019 05:05, Fu, Linjie wrote:
> >> -Original Message-
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> >> Of Fu, Linjie
> >> Sent: Friday, November 16, 2018 16:37
> >> To: FFmpeg development discussions and patches >> de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH v2] ffmpeg: Add option to force a
> >> specific decode format
> >>
> >>> -Original Message-
> >>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> >> Behalf
> >>> Of Mark Thompson
> >>> Sent: Thursday, November 15, 2018 05:48
> >>> To: ffmpeg-devel@ffmpeg.org
> >>> Subject: Re: [FFmpeg-devel] [PATCH v2] ffmpeg: Add option to force a
> >>> specific decode format
> >>>
> >>> On 14/11/18 01:35, Fu, Linjie wrote:
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> >>> Behalf
> > Of Mark Thompson
> > Sent: Wednesday, November 14, 2018 09:11
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v2] ffmpeg: Add option to force a
> > specific decode format
> >
> > On 14/11/18 00:50, Fu, Linjie wrote:
> >>> -Original Message-
> >>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > Behalf
> >>> Of Mark Thompson
> >>> Sent: Wednesday, November 14, 2018 07:44
> >>> To: ffmpeg-devel@ffmpeg.org
> >>> Subject: [FFmpeg-devel] [PATCH v2] ffmpeg: Add option to force a
> > specific
> >>> decode format
> >>>
> >>> Fixes #7519.
> >>> ---
> >>> doc/ffmpeg.texi | 13
> >>> fftools/ffmpeg.c | 10 ++
> >>> fftools/ffmpeg.h | 4
> >>> fftools/ffmpeg_opt.c | 47
> >>>
> >>> 4 files changed, 74 insertions(+)
> >>>
> >>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >>> index 3717f22d42..d127bc0f0d 100644
> >>> --- a/doc/ffmpeg.texi
> >>> +++ b/doc/ffmpeg.texi
> >>> @@ -920,6 +920,19 @@ would be more efficient.
> >>> When doing stream copy, copy also non-key frames found at the
> >>> beginning.
> >>>
> >>> +@item -decode_format[:@var{stream_specifier}]
> >>> @var{pixfmt}[,@var{pixfmt}...] (@emph{input,per-stream})
> >>> +Set the possible output formats to be used by the decoder for this
> > stream.
> >>> +If the decoder does not natively support any format in the given list
> >>> for
> >>> +the input stream then decoding will fail rather than continuing with
> >>> a
> >>> +different format.
> >>> +
> >>> +In general this should not be set - the decoder will select an output
> >>> +format based on the input stream parameters and available
> >>> components,
> >>> and
> >>> +that will be automatically converted to whatever the output
> >> requires.
> >>> It
> >>> +may be useful to force a hardware decoder supporting output in
> > multiple
> >>> +different memory types to pick a desired one, or to ensure that a
> > hardware
> >>> +decoder is used when software fallback is also available.
> >>> +
> >>> @item -init_hw_device
> >>> @var{type}[=@var{name}][:@var{device}[,@var{key=value}...]]
> >>> Initialise a new hardware device of type @var{type} called
> >>> @var{name},
> >>> using the
> >>> given device parameters.
> >>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> >>> index 38c21e944a..c651c8d3a8 100644
> >>> --- a/fftools/ffmpeg.c
> >>> +++ b/fftools/ffmpeg.c
> >>> @@ -598,6 +598,7 @@ static void ffmpeg_cleanup(int ret)
> >>> avsubtitle_free(>prev_sub.subtitle);
> >>> av_frame_free(>sub2video.frame);
> >>> av_freep(>filters);
> >>> +av_freep(>decode_formats);
> >>> av_freep(>hwaccel_device);
> >>> av_freep(>dts_buffer);
> >>>
> >>> @@ -2800,6 +2801,15 @@ static enum AVPixelFormat
> >>> get_format(AVCodecContext *s, const enum AVPixelFormat
> >>> const AVCodecHWConfig *config = NULL;
> >>> int i;
> >>>
> >>> +if (ist->decode_formats) {
> >>> +for (i = 0; ist->decode_formats[i] != AV_PIX_FMT_NONE;
> >>> i++)
> >> {
> >>> +if (ist->decode_formats[i] == *p)
> >>> +break;
> >>> +}
> >>> +if (ist->decode_formats[i] != *p)
> >>> +continue;
> >>> +}
> >>> +
> >>> if (!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL))
> >>> break;
> >>>
> >>> diff --git
Re: [FFmpeg-devel] [PATCH] libavformat: forced PCR pid in mpegts muxer
‐‐‐ Original Message ‐‐‐
On Tuesday, 23 de April de 2019 14:20, Moritz Barsnick wrote:
> On Tue, Apr 23, 2019 at 12:03:11 +, Andreas Håkon wrote:
>
> > In addition, one correction regarding the initialization.
> > Sorry, but first version has an error. This is clean!
>
> I had seen that the option got initialized to 0x, but checked that
> that is never a legally assigned pid (there's a "< 16" check
> somewhere), so figured it would be okay as a default.
>
> > ts_st->discontinuity = ts->flags & MPEGTS_FLAG_DISCONT;
> >
> >
> > - /* update PCR pid by using the first video stream */
> >
> >
> > - if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> >
> >
> > - service->pcr_pid == 0x1fff) {
> >
> >
> >
> > - /* update PCR pid by: forced pid or using the first video stream
> > */
> >
> >
> > - if ((ts->pcr_forced_pid != 0x0010 && ts_st->pid ==
> > ts->pcr_forced_pid) ||
> >
> >
> > - (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> > service->pcr_pid == 0x1fff)) {
> >service->pcr_pid = ts_st->pid;
> >
> >
>
> Okay if pid 0x0010 shall actually never carry the PCR. (Probably
> 0x0010-0x001F if I guess by Wikipedia's list of PIDs properly, but
> then, the article isn't very well written for my understanding, and I
> have no idea about PIDs.)
Hi Moritz,
Pids 0x00-0x10 (0-16 in decimal values) are reserved. So pid 0x10 can't carry
PCR marks.
Then I use this value as the "empty" (aka NOT_SET) value for the parameter. In
this case the
default behaviour is used: that's auto-select first video, or first stream if
no video.
Regards.
A.H.
___
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] libavformat: forced PCR pid in mpegts muxer
On Tue, Apr 23, 2019 at 12:03:11 +, Andreas Håkon wrote:
> In addition, one correction regarding the initialization.
> Sorry, but first version has an error. This is clean!
I had seen that the option got initialized to 0x, but checked that
that is never a legally assigned pid (there's a "< 16" check
somewhere), so figured it would be okay as a default.
> ts_st->discontinuity = ts->flags & MPEGTS_FLAG_DISCONT;
> -/* update PCR pid by using the first video stream */
> -if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> -service->pcr_pid == 0x1fff) {
> +/* update PCR pid by: forced pid or using the first video stream */
> +if ((ts->pcr_forced_pid != 0x0010 && ts_st->pid ==
> ts->pcr_forced_pid) ||
> +(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> service->pcr_pid == 0x1fff)) {
> service->pcr_pid = ts_st->pid;
Okay if pid 0x0010 shall actually never carry the PCR. (Probably
0x0010-0x001F if I guess by Wikipedia's list of PIDs properly, but
then, the article isn't very well written for my understanding, and I
have no idea about PIDs.)
Cheers,
Moritz
___
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 1/2] configure: sort decoder/encoder/filter/... names in alphabet order
> print_in_columns() {
> - cols=$(expr $ncols / 24)
> - cat | tr ' ' '\n' | sort | pr -r "-$cols" -w $ncols -t
> + col_width=24
> + cols=$(expr $ncols / $col_width)
> + rows=$(expr $(expr $# + $cols - 1) / $cols)
> + for row in $(seq $rows); do
> + index=$row
> + line=""
> + fmt=""
> + for col in $(seq $cols); do
> + if [ $index -le $# ]; then
> + eval line='"$line "${'$index'}'
> + fmt="$fmt%-${col_width}s"
> + fi
> + index=$(expr $index + $rows)
> + done
> + printf "$fmt\n" $line
> + done | sed 's/ *$//'
> }
The new code is relatively slow.
On linux it adds ~ 1.5s (1500 ms) to the configure time in both bash and dash
on my system, which is roughly additional ~20% to configure run time.
On Windows this will easily add a minute or more (I didn't test).
Few things to consider:
- print_in_column iterates over a _lot_ of values - hundreds or more.
- Subshells - `$(cmd ...)` are relatively very expensive, especially in hot
(inner) loops, and especially on Windows.
- $(expr ...) can typically be replaced with shell arithmetics - $((...)) .
Your part 2 already uses it, and regardless it's been used in configure
before (in pushvar and popvar), so you should use it where possible.
- `for col in $(seq $cols)` need not invoke `seq` on each iterations to always
produce the same output. You can capture its output once on startup.
- All the places which use the new print_in_columns want the result sorted and
duplicate pipe through `tr` and `sort`. The original version already did
`cat | tr ' ' '\n' | sort` in one place, and you can simply replace it with
`set -- $(cat | tr ' ' '\n' | sort)` to get the values as positional
parameters. This will also keep the interface the same as before and will
reduce the patch size.
And finally, is there a good reason to sort the results in columns and not rows?
As you rightfully mentioned, outputs can span several pages, and when reading
it on screen it might be more convenient to read row by row than column by
column. This could also simplify the new code a lot.
___
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] libavformat: forced PCR pid in mpegts muxer
‐‐‐ Original Message ‐‐‐
On Tuesday, 23 de April de 2019 13:43, Moritz Barsnick wrote:
> On Tue, Apr 23, 2019 at 11:15:40 +, Andreas Håkon wrote:
>
> Nits:
>
> > Subject: libavformat: forced PCR pid in mpegts muxer
>
> "libavformat/mpegtsenc: allow to force the PID carrying the PCR"
> (or something along those lines)
>
> > By default FFmpeg selects the first video stream, or the first one in
> > the service if no video stream is found.
>
> This could also go into the documentation, if it doesn't already say so
> somewhere.
>
> > All then the output audio stream (with pid:202) will host the PCR marks.
>
> "All then the"?
>
> > +Override the pid for carring the PCR clock.
>
> ^
>
>
> Typo: carrying
>
> Thanks,
> Moritz
Thank you Moritz!
All applied.
In addition, one correction regarding the initialization.
Sorry, but first version has an error. This is clean!
Regards.
A.H.
---
From af5a92aa00bb159eb2c967ee8764a2a6ffea1abf Mon Sep 17 00:00:00 2001
From: Andreas Hakon
Date: Tue, 23 Apr 2019 12:55:04 +0100
Subject: [PATCH] libavformat/mpegtsenc: allow to force the PID carrying the PCR
This patch provides a new optional parameter for the mpegtsenc muxer.
The parameter "-force_prc_pid" can be used to override the default PCR pid.
By default FFmpeg selects the first video stream, or the first one in the
service if no video stream is found. With this new parameter any of the pids in
the service can be used to carry the PCR clock.
This can be handy for many reasons.
Example:
$ ffmpeg -i input.mpg -c:v libx264 -c:a mp2 \
-f mpegts -mpegts_pmt_start_pid 200 -streamid 0:201 -streamid 1:202 \
-force_pcr_pid 202 output.ts
Result: output audio stream (with pid:202) will host the PCR marks.
Signed-off-by: Andreas Hakon
---
doc/muxers.texi |5 +
libavformat/mpegtsenc.c | 11 ---
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index 83ae017..e5d81ad 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1590,6 +1590,11 @@ is @code{-1}, which results in shifting timestamps so
that they start from 0.
@item omit_video_pes_length @var{boolean}
Omit the PES packet length for video packets. Default is @code{1} (true).
+@item force_pcr_pid @var{integer}
+Override the pid for carrying the PCR clock.
+By default FFmpeg selects the first video stream, or the first one in
+the service if no video stream is found.
+
@item pcr_period @var{integer}
Override the default PCR retransmission time in milliseconds. Ignored if
variable muxrate is selected. Default is @code{20}.
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index fc0ea22..b741b48 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -96,6 +96,7 @@ typedef struct MpegTSWrite {
int pmt_start_pid;
int start_pid;
+int pcr_forced_pid;
int m2ts_mode;
int reemit_pat_pmt; // backward compatibility
@@ -913,9 +914,9 @@ static int mpegts_init(AVFormatContext *s)
ts_st->first_pts_check = 1;
ts_st->cc = 15;
ts_st->discontinuity = ts->flags & MPEGTS_FLAG_DISCONT;
-/* update PCR pid by using the first video stream */
-if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
-service->pcr_pid == 0x1fff) {
+/* update PCR pid by: forced pid or using the first video stream */
+if ((ts->pcr_forced_pid != 0x0010 && ts_st->pid == ts->pcr_forced_pid)
||
+(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
service->pcr_pid == 0x1fff)) {
service->pcr_pid = ts_st->pid;
pcr_st = st;
}
@@ -960,6 +961,7 @@ static int mpegts_init(AVFormatContext *s)
service->pcr_pid = ts_st->pid;
} else
ts_st = pcr_st->priv_data;
+av_log(s, AV_LOG_VERBOSE, "PCR in pid %d (%d) \n", service->pcr_pid,
st->index);
if (ts->mux_rate > 1) {
service->pcr_packet_period = (int64_t)ts->mux_rate * ts->pcr_period /
@@ -1917,6 +1919,9 @@ static const AVOption options[] = {
{ "mpegts_start_pid", "Set the first pid.",
offsetof(MpegTSWrite, start_pid), AV_OPT_TYPE_INT,
{ .i64 = 0x0100 }, 0x0010, 0x0f00, AV_OPT_FLAG_ENCODING_PARAM },
+{ "force_pcr_pid", "Force the pid carring PCR.",
+ offsetof(MpegTSWrite, pcr_forced_pid), AV_OPT_TYPE_INT,
+ { .i64 = 0x0010 }, 0x0010, 0x1ffe, AV_OPT_FLAG_ENCODING_PARAM },
{ "mpegts_m2ts_mode", "Enable m2ts mode.",
offsetof(MpegTSWrite, m2ts_mode), AV_OPT_TYPE_BOOL,
{ .i64 = -1 }, -1, 1, AV_OPT_FLAG_ENCODING_PARAM },
--
1.7.10.4
___
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] libavformat: forced PCR pid in mpegts muxer
On Tue, Apr 23, 2019 at 11:15:40 +, Andreas Håkon wrote: Nits: > Subject: libavformat: forced PCR pid in mpegts muxer "libavformat/mpegtsenc: allow to force the PID carrying the PCR" (or something along those lines) > By default FFmpeg selects the first video stream, or the first one in > the service if no video stream is found. This could also go into the documentation, if it doesn't already say so somewhere. > All then the output audio stream (with pid:202) will host the PCR marks. "All then the"? > +Override the pid for carring the PCR clock. ^ Typo: carrying Thanks, Moritz ___ 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] libavformat: forced PCR pid in mpegts muxer
New optional parameter “-force_prc_pid” for the mpegtsenc muxer.
Regards.
A.H.
---From ecda0e2f9b0e01a62f06cfb00684d11de6eb6dc2 Mon Sep 17 00:00:00 2001
From: Andreas Hakon
Date: Tue, 23 Apr 2019 12:05:40 +0100
Subject: [PATCH] libavformat: forced PCR pid in mpegts muxer
This patch provides a new optional parameter for the mpegtsenc muxer.
The parameter "-force_prc_pid" can be used to override the default PCR pid.
By default FFmpeg selects the first video stream, or the first one in the
service if no video stream is found. With this new parameter any of the pids in
the service can be used to carry the PCR clock.
This can be handy for many reasons.
Example:
$ ffmpeg -i input.mpg -c:v libx264 -c:a mp2 \
-f mpegts -mpegts_pmt_start_pid 200 -streamid 0:201 -streamid 1:202 \
-force_pcr_pid 202 output.ts
All then the output audio stream (with pid:202) will host the PCR marks.
Signed-off-by: Andreas Hakon
---
doc/muxers.texi |3 +++
libavformat/mpegtsenc.c | 11 ---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index 83ae017..8b6d9d3 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1590,6 +1590,9 @@ is @code{-1}, which results in shifting timestamps so
that they start from 0.
@item omit_video_pes_length @var{boolean}
Omit the PES packet length for video packets. Default is @code{1} (true).
+@item force_pcr_pid @var{integer}
+Override the pid for carring the PCR clock.
+
@item pcr_period @var{integer}
Override the default PCR retransmission time in milliseconds. Ignored if
variable muxrate is selected. Default is @code{20}.
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index fc0ea22..b839b10 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -96,6 +96,7 @@ typedef struct MpegTSWrite {
int pmt_start_pid;
int start_pid;
+int pcr_forced_pid;
int m2ts_mode;
int reemit_pat_pmt; // backward compatibility
@@ -913,9 +914,9 @@ static int mpegts_init(AVFormatContext *s)
ts_st->first_pts_check = 1;
ts_st->cc = 15;
ts_st->discontinuity = ts->flags & MPEGTS_FLAG_DISCONT;
-/* update PCR pid by using the first video stream */
-if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
-service->pcr_pid == 0x1fff) {
+/* update PCR pid by: forced pid or using the first video stream */
+if (ts_st->pid == ts->pcr_forced_pid ||
+(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
service->pcr_pid == 0x1fff)) {
service->pcr_pid = ts_st->pid;
pcr_st = st;
}
@@ -960,6 +961,7 @@ static int mpegts_init(AVFormatContext *s)
service->pcr_pid = ts_st->pid;
} else
ts_st = pcr_st->priv_data;
+av_log(s, AV_LOG_VERBOSE, "PCR in pid %d (%d) \n", service->pcr_pid,
st->index);
if (ts->mux_rate > 1) {
service->pcr_packet_period = (int64_t)ts->mux_rate * ts->pcr_period /
@@ -1917,6 +1919,9 @@ static const AVOption options[] = {
{ "mpegts_start_pid", "Set the first pid.",
offsetof(MpegTSWrite, start_pid), AV_OPT_TYPE_INT,
{ .i64 = 0x0100 }, 0x0010, 0x0f00, AV_OPT_FLAG_ENCODING_PARAM },
+{ "force_pcr_pid", "Force the pid carring PCR.",
+ offsetof(MpegTSWrite, pcr_forced_pid), AV_OPT_TYPE_INT,
+ { .i64 = 0x }, 0x0010, 0x1ffe, AV_OPT_FLAG_ENCODING_PARAM },
{ "mpegts_m2ts_mode", "Enable m2ts mode.",
offsetof(MpegTSWrite, m2ts_mode), AV_OPT_TYPE_BOOL,
{ .i64 = -1 }, -1, 1, AV_OPT_FLAG_ENCODING_PARAM },
--
1.7.10.4
___
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] cuviddec: improved way of finding out if a frame is interlaced or progressive
applied, thanks. smime.p7s Description: S/MIME Cryptographic 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 V2 2/2] lavfi/opencl: add nlmeans_opencl filter
> -Original Message- > From: Song, Ruiling > Sent: Sunday, April 21, 2019 8:18 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: RE: [FFmpeg-devel] [PATCH V2 2/2] lavfi/opencl: add > nlmeans_opencl filter > > > > > -Original Message- > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > Behalf Of > > Mark Thompson > > Sent: Saturday, April 20, 2019 11:08 PM > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH V2 2/2] lavfi/opencl: add > nlmeans_opencl > > filter > > > > On 17/04/2019 03:43, Song, Ruiling wrote: > > >> -Original Message- > > >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > Behalf > > Of > > >> Mark Thompson > > >> Sent: Wednesday, April 17, 2019 5:28 AM > > >> To: ffmpeg-devel@ffmpeg.org > > >> Subject: Re: [FFmpeg-devel] [PATCH V2 2/2] lavfi/opencl: add > > nlmeans_opencl > > >> filter > > >> > > >> On 12/04/2019 16:09, Ruiling Song wrote: > > >>> Signed-off-by: Ruiling Song > > >> > > >> I can't work out where the problem is, but there is something really > weirdly > > >> nondeterministic going on here. > > >> > > >> E.g. > > >> > > >> $ ./ffmpeg_g -y -init_hw_device opencl:0.0 -i ~/video/test/jellyfish-120- > > mbps- > > >> 4k-uhd-hevc-10bit.mkv -an -filter_hw_device opencl0 -vf > > >> > format=yuv420p,hwupload,nlmeans_opencl,hwdownload,format=yuv420p - > > >> frames:v 10 -f framemd5 - > > >> ... > > >> 0, 0, 0,1, 12441600, > > >> 8b8805818076b23ae6f80ec2b5a349d4 > > >> 0, 1, 1,1, 12441600, > > >> 7a7fdaa083dc337cfb6af31b643f30a3 > > >> 0, 2, 2,1, 12441600, > > >> b10ef2a1e5125cc67e262e086f8040b5 > > >> 0, 3, 3,1, 12441600, > > >> c06b53ad90e0357e537df41b63d5b1dc > > >> 0, 4, 4,1, 12441600, > > >> 5aa2da07703859a3dee080847dd17d46 > > >> 0, 5, 5,1, 12441600, > > >> 733364c6be6af825057e905a6092937d > > >> 0, 6, 6,1, 12441600, > > >> 47edae2dec956a582b04babb745d26b0 > > >> 0, 7, 7,1, 12441600, > > >> 4e45fe8268df4298d06a17ab8e46c3e9 > > >> 0, 8, 8,1, 12441600, > > >> 960d722a3f8787c9191299a114c04174 > > >> 0, 9, 9,1, 12441600, > > >> e759c07ee4834a9cf94bfcb4128e7612 > > >> $ ./ffmpeg_g -y -init_hw_device opencl:0.0 -i ~/video/test/jellyfish-120- > > mbps- > > >> 4k-uhd-hevc-10bit.mkv -an -filter_hw_device opencl0 -vf > > >> > format=yuv420p,hwupload,nlmeans_opencl,hwdownload,format=yuv420p - > > >> frames:v 10 -f framemd5 - > > >> 0, 0, 0,1, 12441600, > > >> 8b8805818076b23ae6f80ec2b5a349d4 > > >> [Parsed_nlmeans_opencl_2 @ 0x5557ae580d00] integral image > overflow > > >> 2157538 > > >> 0, 1, 1,1, 12441600, > > >> bce72e10a9f1118940c5a8392ad78ec3 > > >> 0, 2, 2,1, 12441600, > > >> b10ef2a1e5125cc67e262e086f8040b5 > > >> 0, 3, 3,1, 12441600, > > >> c06b53ad90e0357e537df41b63d5b1dc > > >> 0, 4, 4,1, 12441600, > > >> 5aa2da07703859a3dee080847dd17d46 > > >> 0, 5, 5,1, 12441600, > > >> 733364c6be6af825057e905a6092937d > > >> 0, 6, 6,1, 12441600, > > >> 47edae2dec956a582b04babb745d26b0 > > >> 0, 7, 7,1, 12441600, > > >> 4e45fe8268df4298d06a17ab8e46c3e9 > > >> 0, 8, 8,1, 12441600, > > >> 960d722a3f8787c9191299a114c04174 > > >> 0, 9, 9,1, 12441600, > > >> e759c07ee4834a9cf94bfcb4128e7612 > > >> $ ./ffmpeg_g -y -init_hw_device opencl:0.0 -i ~/video/test/jellyfish-120- > > mbps- > > >> 4k-uhd-hevc-10bit.mkv -an -filter_hw_device opencl0 -vf > > >> > format=yuv420p,hwupload,nlmeans_opencl,hwdownload,format=yuv420p - > > >> frames:v 10 -f framemd5 - > > >> 0, 0, 0,1, 12441600, > > >> 8b8805818076b23ae6f80ec2b5a349d4 > > >> 0, 1, 1,1, 12441600, > > >> 7a7fdaa083dc337cfb6af31b643f30a3 > > >> [Parsed_nlmeans_opencl_2 @ 0x557c51fbfe80] integral image overflow > > >> 2098545 > > >> 0, 2, 2,1, 12441600, > > >> 68b390535adc5cfa0f8a7942c42a47ca > > >> 0, 3, 3,1, 12441600, > > >> c06b53ad90e0357e537df41b63d5b1dc > > >> 0, 4, 4,1, 12441600, > > >> 5aa2da07703859a3dee080847dd17d46 > > >> 0, 5, 5,1, 12441600, > > >> 733364c6be6af825057e905a6092937d > > >> 0, 6, 6,1, 12441600, > > >> 47edae2dec956a582b04babb745d26b0 > > >> 0, 7, 7,1, 12441600, > > >> 4e45fe8268df4298d06a17ab8e46c3e9 > > >> 0, 8, 8,1, 12441600, > > >> 960d722a3f8787c9191299a114c04174 > > >> 0, 9, 9,1, 12441600, > > >>
Re: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with CO3 define
‐‐‐ Original Message ‐‐‐
On Tuesday, 23 de April de 2019 9:29, Andreas Håkon
wrote:
> ‐‐‐ Original Message ‐‐‐
> On Tuesday, 23 de April de 2019 8:39, Li, Zhong zhong...@intel.com wrote:
>
> > I belive they are different. If you extend the MARCIO, they are actually:
> > IMHO, your patch is only needed when disable "QSV_HAVE_CO3", but tiket#7839
> > is not root caused now.
> > I will consider to accept it when tiket#7839 is root caused.
>
> Hi Li,
>
> With all due respect.
> You're assuming that:
> "QSV_VERSION_ATLEAST(1, 18)" > "QSV_VERSION_ATLEAST(1, 11)"
> So,
> "QSV_VERSION_ATLEAST(1, 18)" implies that "QSV_VERSION_ATLEAST(1, 11)"
>
> But the CODE doesn't say that!
>
> The code uses:
> "#if QSV_HAVE_CO3"
>
> And then every use of the "co3" member needs to be protected by this clausule.
> Futhermore, this is true in every part of the source code in "qsvenc.c"...
> except at
> two points (I need to review my patch, as I noticed another block not
> protected).
>
> This is a must, even if "QSV_VERSION_ATLEAST(1, 18)" >
> "QSV_VERSION_ATLEAST(1, 11)"
>
> Why?
> Because the option "QSV_HAVE_CO3" can be rewriten (for example, disabled).
>
> So the conclusion is that the code doesn't say "QSV_VERSION_ATLEAST(x)".
>
> Anyway, thanks for discussing it. Stay tuned, while I prepare another patch
> protecting all cases.
>
> Regards.
> A.H.
>
>
Hi Li,
Here the updated patch.
I hope you agree with it!
Regards.
A.H.
---
From 211c6704e4e4272221fbc8c908b99307b11e1800 Mon Sep 17 00:00:00 2001
From: Andreas Hakon
Date: Tue, 23 Apr 2019 09:25:15 +0100
Subject: [PATCH] libavcodec: QSV protect GPB code with CO3 define [v2]
This patch protects some code using the member "co3" inside the qsvenc.c file.
The first block is a simple debug message, so it doesn't generates troubles.
The second block is an initialization using of the "extco3" for the QVBR mode.
After this patch it's safe to enable/disable some options in the source code
without triggering compilation errors.
Signed-off-by: Andreas Hakon
---
libavcodec/qsvenc.c |4
1 file changed, 4 insertions(+)
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index a03ab69..feedceb 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -267,10 +267,12 @@ static void dump_video_param(AVCodecContext *avctx,
QSVEncContext *q,
#endif
#endif
+#if QSV_HAVE_CO3
#if QSV_HAVE_GPB
if (avctx->codec_id == AV_CODEC_ID_HEVC)
av_log(avctx, AV_LOG_VERBOSE,"GPB: %s\n", print_threestate(co3->GPB));
#endif
+#endif
if (avctx->codec_id == AV_CODEC_ID_H264) {
av_log(avctx, AV_LOG_VERBOSE, "Entropy coding: %s;
MaxDecFrameBuffering: %"PRIu16"\n",
@@ -598,9 +600,11 @@ static int init_video_param(AVCodecContext *avctx,
QSVEncContext *q)
q->param.mfx.MaxKbps = max_bitrate_kbps /
brc_param_multiplier;
q->param.mfx.BRCParamMultiplier = brc_param_multiplier;
#if QSV_HAVE_QVBR
+#if QSV_HAVE_CO3
if (q->param.mfx.RateControlMethod == MFX_RATECONTROL_QVBR)
q->extco3.QVBRQuality = av_clip(avctx->global_quality, 0, 51);
#endif
+#endif
break;
case MFX_RATECONTROL_CQP:
quant = avctx->global_quality / FF_QP2LAMBDA;
--
1.7.10.4
___
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] lavc/vaapi_encode_h264: add support for maxframesize
> -Original Message-
> From: myp...@gmail.com [mailto:myp...@gmail.com]
> Sent: Tuesday, April 23, 2019 13:42
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Cc: Fu, Linjie
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode_h264: add support
> for maxframesize
>
> On Tue, Apr 23, 2019 at 11:29 AM Linjie Fu wrote:
> >
> > Add support for max frame size:
> > - max_frame_size (bytes) to indicate the allowed max frame size.
> > - pass_num to indicate number of passes.
> > - delta_qp to indicate adjust qp value.
> >
> > Currently only AVC encoder can support this settings in multiple pass case.
> > If the frame size exceeds, the encoder will do more pak passes to adjust
> the
> > QP value to control the frame size.
> >
> > Set Default num_passes to 4 (1~4), set delta_qp[4] = {1, 1, 1, 1}, use
> > new_qp for encoder if frame size exceeds the limitation:
> > new_qp = base_qp + delta_qp[0] + delta_qp[1] + ...
> >
> > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo \
> > -v verbose -s:v 352x288 -i ./input.yuv -vf format=nv12,hwupload \
> > -c:v h264_vaapi -profile:v main -g 30 -bf 3 -max_frame_size 4 \
> > -pass_num 2 -delta_qp 2 -vframes 100 -y ./max_frame_size.h264
> >
> Some question list as follow：
>
> 1. Can I change delta_qp per pass, e,g, 4 pass 1, 2, 3, 4, use
> delta_qp[4] like: 1, 2, 4, 8?
Yes, it's available.
Actually I considered this whether to expose the detailed settings for the user,
And end up with current solution:
- setting max_frame_size only
will use default 4 passes with delta_qp = {1, 1, 1, 1}
- setting max_frame_size with detailed pass_num and delta_qp
Can be customized by user.
Not splitting delta_qp setting for each pass is trying to make it easier for
users.
> 2. So let's think about the limiting case, if we setting
> max_frame_size = 1 for 1080P, what's the action for this driver?
Maybe the commit message is a little bit confusing,
driver will adjust add delta_qp to original qp to decrease the frame size for
each pass:
new_qp = base_qp + delta_qp[0]
If the frame size is still too big, will try 2 passes
new_qp = base_qp + delta_qp[0] + delta_qp[1]
If the frame size is still too big after 4 passes qp modification, driver will
use max qp allowed by the sum of delta_qp;
new_qp = base_qp + delta_qp[0] + delta_qp[1] + delta_qp[2] +
delta_qp[3]
> 3. Maybe we can hide the pass_num and delta_qp, only export the
> max_frame_size for this case? I don't think export the driver QP
> adjustment detail logic to user space is good idea, user will
> confused about to how to set/adjust pass_num/delta_qp per pass.
Similar with 1, I think a default path with a valuable set of parameters(Needs
advice), together with a customizing method is good.
> 4. Missing docs
Will refine the commit message, and a doc if still needed.
> 5. What's the relationship about other bit rate control like VBR or MBBRC ?
IMHO, max frame size will modify the qp value set by bit rate control methods,
and trying to meet the size limitation for each frame.
> 6. Only 264 encoder support this feature? What platform have you tested?
Yes, only AVC encoder can be supported by iHD driver, and tested in KBL.
Thanks for the comment.
Linjie
___
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] libavcodec: QSV protect GPB code with CO3 define
‐‐‐ Original Message ‐‐‐ On Tuesday, 23 de April de 2019 8:39, Li, Zhong wrote: > > I belive they are different. If you extend the MARCIO, they are actually: > > > IMHO, your patch is only needed when disable "QSV_HAVE_CO3", but tiket#7839 > is not root caused now. > I will consider to accept it when tiket#7839 is root caused. > Hi Li, With all due respect. You're assuming that: "QSV_VERSION_ATLEAST(1, 18)" > "QSV_VERSION_ATLEAST(1, 11)" So, "QSV_VERSION_ATLEAST(1, 18)" implies that "QSV_VERSION_ATLEAST(1, 11)" But the CODE doesn't say that! The code uses: "#if QSV_HAVE_CO3" And then ***every*** use of the "co3" member needs to be protected by this clausule. Futhermore, this is true in every part of the source code in "qsvenc.c"... except at two points (I need to review my patch, as I noticed another block not protected). This is a must, even if "QSV_VERSION_ATLEAST(1, 18)" > "QSV_VERSION_ATLEAST(1, 11)" Why? Because the option "QSV_HAVE_CO3" can be rewriten (for example, disabled). So the conclusion is that the code doesn't say "QSV_VERSION_ATLEAST(x)". Anyway, thanks for discussing it. Stay tuned, while I prepare another patch protecting all cases. Regards. A.H. --- ___ 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] libavformat: fix inputs initialization in mpegts muxer with filters
‐‐‐ Original Message ‐‐‐ On Tuesday, 23 de April de 2019 1:07, Michael Niedermayer wrote: > On Mon, Apr 22, 2019 at 11:42:57AM +, Andreas Håkon wrote: > > > > Please, revise the code! I hope you understand it when you look at my > > descriptions. > > I would prefer to have some testcase that shows a problem > what you write basically sounds like > there are 2 similar pieces of code but they differ, so you change > one assuming the other is correct. Maybe the change is correct but > the explanation is insufficient > Hi Michael, I agree that a testcase is preferable. However, I can use the Mathematical Demonstration by reduction to the absurd: - The only difference between block A and B (the two "if () ... else if () ...") is that the first one is executed when a filter graph is running. - In the second block (block B) the "ost->inputs_done = 1;" is executed prior to return. - In the first block (bloc A) is not executed. Why is then executed in the Block B? If the only difference is the filter graph, and nothing related to inputs, then only one of these two can be true: 1) Or the Block B doesn't needs the code "ost->inputs_done = 1;" 2) Or the Block A needs it too. And I don't think the option 1 is true. So by reduction to the absurd is demonstrated. Regards. A.H. --- ___ 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] libavformat: fix copyts and muxrate in mpegts muxer
‐‐‐ Original Message ‐‐‐
On Tuesday, 23 de April de 2019 0:36, Michael Niedermayer
wrote:
>
> > From c59569ca9426fef455edabfa648cb2ff678c1640 Mon Sep 17 00:00:00 2001
> > From: Andreas Hakon andreas.ha...@protonmail.com
> > Date: Fri, 19 Apr 2019 09:32:33 +0100
> > Subject: [PATCH] libavformat: fix copyts and muxrate in mpegts muxer v2
> > When using "-copyts" and "-muxrate" with the mpegts muxer the muxing fails
> > because the member "first_pcr" of the MpegTSWrite struct isn't initialized
> > with a correct timestamp.
> > The behaviour of the error is an infinite loop created in the function
> > mpegts_write_pes() because the code never writes PES packets.
> > This patch resolves the problem initializing the "first_pcr" with a value
> > derived from the first DTS value seen.
> >
> > Signed-off-by: Andreas Hakon andreas.ha...@protonmail.com
> >
> > --
> >
> > libavformat/mpegtsenc.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> > diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> > index fc0ea22..858b0d7 100644
> > --- a/libavformat/mpegtsenc.c
> > +++ b/libavformat/mpegtsenc.c
> > @@ -971,6 +971,9 @@ static int mpegts_init(AVFormatContext *s)
> >
> > if (ts->copyts < 1)
> > ts->first_pcr = av_rescale(s->max_delay, PCR_TIME_BASE,
> > AV_TIME_BASE);
> > +else
> > +ts->first_pcr = AV_NOPTS_VALUE;
> > + } else {
> > /* Arbitrary values, PAT/PMT will also be written on video key
> > frames */
> > ts->sdt_packet_period = 200;
> > @@ -1186,12 +1189,16 @@ static void mpegts_write_pes(AVFormatContext *s,
> > AVStream st,
> > int64_t pcr = -1; / avoid warning */int64_t delay =
> > av_rescale(s->max_delay, 9, AV_TIME_BASE);
> > int force_pat = st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && key
> > && !ts_st->prev_payload_key;
> > + int last_payload_size = 0;
> > av_assert0(ts_st->payload != buf || st->codecpar->codec_type !=
> > AVMEDIA_TYPE_VIDEO);
> > if (ts->flags & MPEGTS_FLAG_PAT_PMT_AT_FRAMES &&
> > st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
> > force_pat = 1;
> > }
> >
> > + if (ts->mux_rate > 1 && dts != AV_NOPTS_VALUE && ts->first_pcr ==
> > AV_NOPTS_VALUE)
> > + ts->first_pcr = (dts * 300) - av_rescale(s->max_delay,
> > PCR_TIME_BASE, AV_TIME_BASE);
> > +
>
> if this doesnt execute first_pcr could remain AV_NOPTS_VALUE, this looks
> a bit fragile to me as there is code using first_pcr with implicit assumtation
> that it is not AV_NOPTS_VALUE in get_pcr().
> is something preventing this combination ?
After reading it, I don't see any reason to this assumption...
But, you need to understand that the function "get_pcr()" ***uses*** the
"first_pcr" value.
Check it at:
https://github.com/FFmpeg/FFmpeg/blob/694d9d53685333771823285457bdd1ef1480eafc/libavformat/mpegtsenc.c#L747
So, you can't use "get_pcr()" to calculate "first_pcr" or you'll go to a loop!
So the code is correct here.
>
> > is_start = 1;
> > while (payload_size > 0) {
> > retransmit_si_info(s, force_pat, dts);
> >
>
> > @@ -1209,12 +1216,13 @@ static void mpegts_write_pes(AVFormatContext *s,
> > AVStream *st,
> > }
> >
> > if (ts->mux_rate > 1 && dts != AV_NOPTS_VALUE &&
> > - (dts - get_pcr(ts, s->pb) / 300) > delay) {
> > + last_payload_size != payload_size && (dts - get_pcr(ts,
> > s->pb) / 300) > delay) {
> >/* pcr insert gets priority over null packet insert */
> >if (write_pcr)
> >mpegts_insert_pcr_only(s, st);
> >else
> >mpegts_insert_null_packet(s);
> > + last_payload_size = payload_size;
> >
> >
>
> why is the check on payload_size needed ? (it is not for the testcase)
> the commit message also seems not to explain this part
This is required because the loop needs to break (the cause of the infinite
loop: the *bug* is here).
Let me to explain in more detail:
- The loop is created because the code enters in the block because (don't care
about the rest):
"(dts - get_pcr(ts, s->pb) / 300) > delay)". And "get_pcr()" uses the
"first_pcr". So, for this
reason we need to properly initialize it in the previous block. And this is the
first part of the
solution for the bug.
- But the second part is how to break the loop. The code really needs to be
executed if the
difference between the DTS and the PCR is greater than the fixed delay.
However, nothing prevents to
re-enter in the code another time, generating the loop. The solution to break
it is to check if
the "last_payload_size" is different from the current one. If this is true,
then it's true that
something is writed in the stream. And in this case is then valid to re-enter
in the block.
But in the opposite case when nothing is writen, then we don't need to enter in
the block.
So, for this reason it's
Re: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with CO3 define
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Andreas Håkon > Sent: Monday, April 22, 2019 8:21 PM > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with > CO3 define > > > ‐‐‐ Original Message ‐‐‐ > On Monday, 22 de April de 2019 13:33, Li, Zhong > wrote: > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > > > Behalf Of Andreas Håkon via ffmpeg-devel > > > Sent: Thursday, April 18, 2019 6:11 PM > > > To: FFmpeg development discussions and patches > > > ffmpeg-devel@ffmpeg.org > > > Cc: Andreas Håkon andreas.ha...@protonmail.com > > > Subject: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code > > > with > > > CO3 define > > > Hi, > > > In response to this ticket I provide the first part of the patch: > > > https://trac.ffmpeg.org/ticket/7839 > > > This QSV_HAVE_GPB code needs to be protected by QSV_HAVE_CO3. > > > Regards. > > > A.H. > > > > Why it is must? It is impossible that QSV_HAVE_GPB is enabled but > QSV_HAVE_CO3 is not enabled. > > QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API > V1.11. > > > > Hi Li, > > > Why it is must? > > Let me to explain why: > > - The "#if QSV_HAVE_GPB" only appears two times inside > "/libavcodec/qsvenc.c": > 1. > https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/qsvenc.c#L75 > 6 > 2. > https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/qsvenc.c#L27 > 0 > > In the first occurrence (line #756) the code is protected by one "#if > QSV_HAVE_CO3". This is necessary because if QSV_HAVE_CO3 is not enabled > then the code fails when using "extco3.GPB". The original author (which I > think was you) included the test with sound common sense. I belive they are different. If you extend the MARCIO, they are actually: Case 1: #if QSV_VERSION_ATLEAST(1, 11) q->extco3.Header.BufferId = MFX_EXTBUFF_CODING_OPTION3; q->extco3.Header.BufferSz = sizeof(q->extco3); #if QSV_VERSION_ATLEAST(1, 18) if (avctx->codec_id == AV_CODEC_ID_HEVC) q->extco3.GPB = q->gpb ? MFX_CODINGOPTION_ON : MFX_CODINGOPTION_OFF; #endif q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer *)>extco3; #endif And Case2: #if QSV_VERSION_ATLEAST(1, 18) if (avctx->codec_id == AV_CODEC_ID_HEVC) av_log(avctx, AV_LOG_VERBOSE,"GPB: %s\n", print_threestate(co3->GPB)); #endif You must add "#if QSV_VERSION_ATLEAST(1, 11)" for case 1, else " q->extco3.Header.BufferId = MFX_EXTBUFF_CODING_OPTION3;" is broken. But you don't need change Case 2 to be: #if QSV_VERSION_ATLEAST(1, 11) #if QSV_VERSION_ATLEAST(1, 18) if (avctx->codec_id == AV_CODEC_ID_HEVC) av_log(avctx, AV_LOG_VERBOSE,"GPB: %s\n", print_threestate(co3->GPB)); #endif #endif > In the second occurrence (line #270) where my patch is applied, the code > isn't protected. The reason is because this code is changed **after**. And > the developer forgot to protect it. > > So my patch is a simple fixing. > > > It is impossible that QSV_HAVE_GPB is enabled but QSV_HAVE_CO3 is not > enabled. > > QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API > V1.11. > > This isn't true in all cases. As described in > "https://trac.ffmpeg.org/ticket/7839; > one current bug breaks the "mpeg2_qsv" encoder. One solution is to > MANUALLY disable the QVBR. This can be done with a simple "#define > QSV_HAVE_QVBR 0". Even if you compile with a recent version of the SDK > > v1.11. > > But the problem is then that this implies too to disable "QSV_HAVE_CO3". > And doing this the code doesn't compile as this patch isn't applied. > > So, the best solution is to merge this patch. It enables then the option to > disable manually what you like, and the code compiles clean. > > Please, accept the patch. IMHO, your patch is only needed when disable "QSV_HAVE_CO3", but tiket#7839 is not root caused now. I will consider to accept it when tiket#7839 is root caused. ___ 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".
