[FFmpeg-devel] [PATCH] scale_npp: fix passthrough mode

2016-10-24 Thread Yogender Gupta
The pass through mode is broken, since error is returned when none of the 
stages is needed. In such a case only 0 should be returned.

Try command line with i/p resolution = o/p resolution to observe the issue.

Sample Command Line : ffmpeg -y -hwaccel cuvid -c:v h264_cuvid -vsync 0 -i 
amazing_10.mp4 -vf scale_npp=1920:1072 -vcodec h264_nvenc tmp0.mp4 (amazing.mp4 
should be 1920x1072 in this case)

Thanks,
Yogender


---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---


0001-scale_npp-fix-passthrough-mode.patch
Description: 0001-scale_npp-fix-passthrough-mode.patch
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avcodec: validate codec parameters in avcodec_parameters_to_context

2016-10-24 Thread Andreas Cadhalpun
This should reduce the impact of a demuxer (or API user) setting bogus
codec parameters.

Suggested-by: wm4
Signed-off-by: Andreas Cadhalpun 
---
 libavcodec/utils.c | 82 +-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 87de15f..9977ffd 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -4227,8 +4227,20 @@ int avcodec_parameters_to_context(AVCodecContext *codec,
 codec->codec_id   = par->codec_id;
 codec->codec_tag  = par->codec_tag;
 
+if (par->bit_rate < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid bit rate %"PRId64"\n", 
par->bit_rate);
+return AVERROR(EINVAL);
+}
 codec->bit_rate  = par->bit_rate;
+if (par->bits_per_coded_sample < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid bits per coded sample %d\n", 
par->bits_per_coded_sample);
+return AVERROR(EINVAL);
+}
 codec->bits_per_coded_sample = par->bits_per_coded_sample;
+if (par->bits_per_raw_sample < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid bits per raw sample %d\n", 
par->bits_per_raw_sample);
+return AVERROR(EINVAL);
+}
 codec->bits_per_raw_sample   = par->bits_per_raw_sample;
 codec->profile   = par->profile;
 codec->level = par->level;
@@ -4236,42 +4248,110 @@ int avcodec_parameters_to_context(AVCodecContext 
*codec,
 switch (par->codec_type) {
 case AVMEDIA_TYPE_VIDEO:
 codec->pix_fmt= par->format;
+if ( (par->width || par->height) && av_image_check_size(par->width, 
par->height, 0, codec) < 0)
+return AVERROR(EINVAL);
 codec->width  = par->width;
 codec->height = par->height;
 codec->field_order= par->field_order;
+if (par->color_range < 0 || par->color_range > AVCOL_RANGE_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid color range %d\n", 
par->color_range);
+return AVERROR(EINVAL);
+}
 codec->color_range= par->color_range;
+if (par->color_primaries < 0 || par->color_primaries > AVCOL_PRI_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid color primaries %d\n", 
par->color_primaries);
+return AVERROR(EINVAL);
+}
 codec->color_primaries= par->color_primaries;
+if (par->color_trc < 0 || par->color_trc > AVCOL_TRC_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid color transfer 
characteristics %d\n", par->color_trc);
+return AVERROR(EINVAL);
+}
 codec->color_trc  = par->color_trc;
+if (par->color_space < 0 || par->color_space > AVCOL_SPC_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid color space %d\n", 
par->color_space);
+return AVERROR(EINVAL);
+}
 codec->colorspace = par->color_space;
+if (par->chroma_location < 0 || par->chroma_location > 
AVCHROMA_LOC_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid chroma location %d\n", 
par->chroma_location);
+return AVERROR(EINVAL);
+}
 codec->chroma_sample_location = par->chroma_location;
+if (par->sample_aspect_ratio.num < 0 || par->sample_aspect_ratio.den < 
0) {
+av_log(codec, AV_LOG_ERROR, "Invalid sample aspect ratio %d/%d\n",
+   par->sample_aspect_ratio.num, par->sample_aspect_ratio.den);
+return AVERROR(EINVAL);
+}
 codec->sample_aspect_ratio= par->sample_aspect_ratio;
+if (par->video_delay < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid video delay %d\n", 
par->video_delay);
+return AVERROR(EINVAL);
+}
 codec->has_b_frames   = par->video_delay;
 break;
 case AVMEDIA_TYPE_AUDIO:
+if (par->format < -1 || par->format > AV_SAMPLE_FMT_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid sample format %d\n", 
par->format);
+return AVERROR(EINVAL);
+}
 codec->sample_fmt   = par->format;
 codec->channel_layout   = par->channel_layout;
+if (par->channels < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid number of channels %d\n", 
par->channels);
+return AVERROR(EINVAL);
+}
 codec->channels = par->channels;
+if (par->sample_rate < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid sample rate %d\n", 
par->sample_rate);
+return AVERROR(EINVAL);
+}
 codec->sample_rate  = par->sample_rate;
+if (par->block_align < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid block align %d\n", 
par->block_align);
+return AVERROR(EINVAL);
+}
 codec->block_align  = par->block_align;
+if (par->frame_size < 0) {
+

Re: [FFmpeg-devel] [PATCH] mov: add option to ignore moov atoms which are detected in free atoms, so apps can have flexibility to use moov atom not in free atoms as default.

2016-10-24 Thread Zhenni Huang
Hi Carl,

Thanks for your reply. Setting strict_std_compliance to 2 could help in
this case. However, as it is a global flag, it could influence other parts
in demuxers. It is preferable if we can have control of whether to use moov
in free with one separate flag.

Thanks,
Zhenni

On Mon, Oct 24, 2016 at 2:54 PM, Carl Eugen Hoyos 
wrote:

> 2016-10-24 23:11 GMT+02:00 Zhenni Huang  ffmpeg.org>:
>
> [...]
>
> Does the following inlined patch help you?
>
> Carl Eugen
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 357d800..ed099fc 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4884,6 +4884,7 @@
>  a.type = avio_rl32(pb);
>  if (a.type == MKTAG('f','r','e','e') &&
>  a.size >= 8 &&
> +c->fc->strict_std_compliance < FF_COMPLIANCE_VERY_STRICT
> &&
>  c->moov_retry) {
>  uint8_t buf[8];
>  uint32_t *type = (uint32_t *)buf + 1;
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Use the correct timescale when seeking for audio.

2016-10-24 Thread Michael Niedermayer
On Sun, Oct 23, 2016 at 10:41:52PM -0700, Sasi Inguva wrote:
> Attaching the fate sample.

uploaded

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] libavcodec : add psd image file decoder

2016-10-24 Thread Carl Eugen Hoyos
2016-10-24 22:21 GMT+02:00 Martin Vignali :
> Hello,
>
> In attach new patchs.

> /* reorganize uncompress data. Each channel is stored one after the other */

(Sorry if I misread the code.)
Did you see AV_PIX_FMT_GBRP, AV_PIX_FMT_GBRAP, AV_PIX_FMT_GBRP16
and AV_PIX_FMT_GBRAP16?
And the more I think of it, the more likely it seems that (GBR) 9, 10, 12 and 14
could also be of use.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread Hendrik Leppkes
On Mon, Oct 24, 2016 at 10:31 PM, Ronald S. Bultje  wrote:
> Hi,
>
> On Mon, Oct 24, 2016 at 4:26 PM, Henrik Gramner  wrote:
>
>> On Mon, Oct 24, 2016 at 9:59 PM, Ronald S. Bultje 
>> wrote:
>> > Good idea to reference Hendrik Gramner here, who keeps insisting we get
>> rid
>> > of all MMX code in ffmpeg (at least as an option) for future Intel CPUs
>> in
>> > which MMX will be deprecated.
>>
>> Replacing MMX with SSE2 is indeed the most "proper" fix in my opinion,
>> but it's a fair amount of work and not done in an evening.
>>
>> The fact that a lot of assembly lacks unit tests is certainly not
>> helping in that regard.
>>
>> Some MMX instructions are slower than the equivalent SSE2 code on
>> Skylake. Intel hasn't officially commented on (as far as I know at
>> least) if we should expect this trend to continue, but they certainly
>> seem to treat MMX as legacy.
>>
>> I doubt they would completely remove support for it though, backwards
>> compatibility is a big selling-point for x86.
>
>
> Well, it gives us another way of fixing this issue (on x86-64 only): have
> sse2 implementations for all code that has a mmx (register) path right now.
>

I don't think the argument for pre-sse2 CPUs is that strong on 32-bit
systems, either.
I wouldn't necessarily limit this to x86-64 at that, considering its
probably another half year at least, probably longer, until such a
conversion could be completed.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] mov: add option to ignore moov atoms which are detected in free atoms, so apps can have flexibility to use moov atom not in free atoms as default.

2016-10-24 Thread Carl Eugen Hoyos
2016-10-24 23:11 GMT+02:00 Zhenni Huang :

[...]

Does the following inlined patch help you?

Carl Eugen

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 357d800..ed099fc 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4884,6 +4884,7 @@
 a.type = avio_rl32(pb);
 if (a.type == MKTAG('f','r','e','e') &&
 a.size >= 8 &&
+c->fc->strict_std_compliance < FF_COMPLIANCE_VERY_STRICT &&
 c->moov_retry) {
 uint8_t buf[8];
 uint32_t *type = (uint32_t *)buf + 1;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] img2: added support for %t output pattern

2016-10-24 Thread Roger Pack
On 10/16/16, Michael Niedermayer  wrote:
> On Mon, Oct 10, 2016 at 02:56:24PM -0600, Roger Pack wrote:
>> On 9/22/16, Roger Pack  wrote:
>> > On 1/4/12, Yuval Adam  wrote:
>> >> From: Yuval Adam 
>> >>
>> >> The image2 muxer now supports timestamps in output filenames.
>> >> When used in an output patterm '%t' will be replaced with the frames
>> >> timestamp in hours, minutes and seconds (hh:mm:ss).
>> >
>> > A somewhat updated (but not yet cleaned up) revision:
>> >
>> > https://gist.github.com/rdp/e518616f2a702367ae5a922b56e09e04
>> >
>> > see also https://trac.ffmpeg.org/ticket/1452
>>
>> OK attached is the "cleaned up" patch, ready for review/commit.
>>
>> how to test:
>> (apply then) run this:
>>
>> ./ffmpeg -i input -copyts -vsync vfr temp/abc-%d-%t.jpeg
>> and compare filenames with the timestamps from video packets of
>> ffprobe -show_packets.
>>
>> Probably a better way would have been to mix it into
>> av_bprint_strftime however I wasn't sure how to use that within
>> libavformat/utils.c av_get_frame_filename2
>>
>> Adam's initial patch
>> (https://github.com/yuvadm/FFmpeg/commit/0eb002821a2076cb3593c823399aeef9fdd29525)
>> also deprecated av_get_frame_filename
>>
>> but I wasn't sure if we wanted that here or not so didn't include it.
>> Thank you for your consideration.
>> -roger-
>
>>  doc/muxers.texi|   19 ---
>>  libavformat/avformat.h |3 ++-
>>  libavformat/hlsenc.c   |6 +++---
>>  libavformat/img2enc.c  |7 +--
>>  libavformat/utils.c|   36 
>>  5 files changed, 58 insertions(+), 13 deletions(-)
>> 06950fc8ba5a9163ffb838a2bff9933e69255b41
>> 0001-img2-encoder-allow-t-in-filename-based-on-patch-from.patch
>> From 11deddfacc595c43a4f542fffe5e90b142e39c85 Mon Sep 17 00:00:00 2001
>> From: rogerdpack 
>> Date: Mon, 10 Oct 2016 14:50:20 -0600
>> Subject: [PATCH] img2 encoder: allow %t in filename, based on patch from
>> Yuval
>>  Adam
>>
>> Signed-off-by: rogerdpack 
>> ---
>>  doc/muxers.texi| 19 ---
>>  libavformat/avformat.h |  3 ++-
>>  libavformat/hlsenc.c   |  6 +++---
>>  libavformat/img2enc.c  |  7 +--
>>  libavformat/utils.c| 36 
>>  5 files changed, 58 insertions(+), 13 deletions(-)
>>
>> diff --git a/doc/muxers.texi b/doc/muxers.texi
>> index 9ec2e31..6fff966 100644
>> --- a/doc/muxers.texi
>> +++ b/doc/muxers.texi
>> @@ -619,6 +619,12 @@ If the pattern contains "%d" or "%0@var{N}d", the
>> first filename of
>>  the file list specified will contain the number 1, all the following
>>  numbers will be sequential.
>>
>> +If the pattern contains "%t", the frame's timestamps will be inserted
>> +in the filename like "00.00.00.000" for hours, minutes, seconds,
>> +and milliseconds.
>> +
>> +The "%t" and "%d" patterns may be used simultaneously.
>> +
>>  The pattern may contain a suffix which is used to automatically
>>  determine the format of the image files to write.
>>
>> @@ -635,7 +641,7 @@ The following example shows how to use
>> @command{ffmpeg} for creating a
>>  sequence of files @file{img-001.jpeg}, @file{img-002.jpeg}, ...,
>>  taking one image every second from the input video:
>>  @example
>> -ffmpeg -i in.avi -vsync 1 -r 1 -f image2 'img-%03d.jpeg'
>> +ffmpeg -i in.avi -vsync cfr -r 1 -f image2 'img-%03d.jpeg'
>>  @end example
>>
>>  Note that with @command{ffmpeg}, if the format is not specified with the
>> @@ -643,12 +649,12 @@ Note that with @command{ffmpeg}, if the format is
>> not specified with the
>>  format, the image2 muxer is automatically selected, so the previous
>>  command can be written as:
>>  @example
>> -ffmpeg -i in.avi -vsync 1 -r 1 'img-%03d.jpeg'
>> +ffmpeg -i in.avi -vsync cfr -r 1 'img-%03d.jpeg'
>>  @end example
>>
>>  Note also that the pattern must not necessarily contain "%d" or
>>  "%0@var{N}d", for example to create a single image file
>> -@file{img.jpeg} from the input video you can employ the command:
>> +@file{img.jpeg} from the start of the input video you can employ the
>> command:
>>  @example
>>  ffmpeg -i in.avi -f image2 -frames:v 1 img.jpeg
>>  @end example
>> @@ -664,6 +670,13 @@ can be used:
>>  ffmpeg -f v4l2 -r 1 -i /dev/video0 -f image2 -strftime 1
>> "%Y-%m-%d_%H-%M-%S.jpg"
>>  @end example
>>
>> +The following example uses the timestamp parameter to generate one
>> +image file per video frame from the input, and name it including its
>> original
>> +timestamp.
>> +@example
>> +ffmpeg -i in.avi -vsync vfr -copyts img-%t.jpg
>> +@end example
>> +
>>  @subsection Options
>>
>>  @table @option
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 057f8c5..4eeb6f4 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -2744,10 +2744,11 @@ void av_dump_format(AVFormatContext *ic,
>>   * @param path numbered sequence 

Re: [FFmpeg-devel] PATCH: dshow prevent some windows 10 anniv. ed crashes

2016-10-24 Thread Roger Pack
On 9/5/16, Roger Pack  wrote:
> On 9/4/16, Carl Eugen Hoyos  wrote:
>> Hi!
>>
>> 2016-08-20 12:09 GMT+02:00 Timo Rothenpieler :
>>> On 8/19/2016 3:28 PM, Roger Pack wrote:
 No complaints, would someone please push it for me? Sorry still
 haven't figured out the key thing yet.
>>>
>>> pushed
>>
>> Did this fix ticket #5775?

OK I heard a rumor microsoft had fixed it on their end, so feel free
to close that ticket now.
Thanks.

> Not entirely, still readying another patch FWIW.
>
>> Carl Eugen
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] mov: add option to ignore moov atoms which are detected in free atoms, so apps can have flexibility to use moov atom not in free atoms as default.

2016-10-24 Thread Zhenni Huang
From: liangsi 

---
 libavformat/isom.h | 1 +
 libavformat/mov.c  | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 2246fed..6824f7e 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -214,6 +214,7 @@ typedef struct MOVContext {
 int use_absolute_path;
 int ignore_editlist;
 int ignore_chapters;
+int ignore_moov_atom_in_free;
 int seek_individually;
 int64_t next_root_atom; ///< offset of the next root atom
 int export_all;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index a15c8d1..1d80c09 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4848,7 +4848,8 @@ static int mov_read_default(MOVContext *c, AVIOContext 
*pb, MOVAtom atom)
 if (atom.size >= 8) {
 a.size = avio_rb32(pb);
 a.type = avio_rl32(pb);
-if (a.type == MKTAG('f','r','e','e') &&
+if (!c->ignore_moov_atom_in_free && 
+a.type == MKTAG('f','r','e','e') &&
 a.size >= 8 &&
 c->moov_retry) {
 uint8_t buf[8];
@@ -5926,6 +5927,8 @@ static const AVOption mov_options[] = {
 0, 1, FLAGS},
 {"ignore_chapters", "", OFFSET(ignore_chapters), AV_OPT_TYPE_BOOL, {.i64 = 
0},
 0, 1, FLAGS},
+{"ignore_moov_atom_in_free", "", OFFSET(ignore_moov_atom_in_free), 
AV_OPT_TYPE_BOOL, 
+{.i64 = 0}, 0, 1, FLAGS},
 {"use_mfra_for",
 "use mfra for fragment timestamps",
 OFFSET(use_mfra_for), AV_OPT_TYPE_INT, {.i64 = FF_MOV_FLAG_MFRA_AUTO},
-- 
2.8.0.rc3.226.g39d4020

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/2] ffmpeg: move some unrelated code out of a filter loop

2016-10-24 Thread Clément Bœsch
---
Not sure if the chunk is even needed
---
 ffmpeg.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index 3b91710..e8088c0 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2317,10 +2317,9 @@ static int decode_video(InputStream *ist, AVPacket *pkt, 
int *got_output, int eo
 }
 
 frame_sample_aspect= av_opt_ptr(avcodec_get_frame_class(), decoded_frame, 
"sample_aspect_ratio");
+if (!frame_sample_aspect->num)
+*frame_sample_aspect = ist->st->sample_aspect_ratio;
 for (i = 0; i < ist->nb_filters; i++) {
-if (!frame_sample_aspect->num)
-*frame_sample_aspect = ist->st->sample_aspect_ratio;
-
 if (i < ist->nb_filters - 1) {
 f = ist->filter_frame;
 err = av_frame_ref(f, decoded_frame);
-- 
2.10.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread Ronald S. Bultje
Hi,

On Mon, Oct 24, 2016 at 4:26 PM, Henrik Gramner  wrote:

> On Mon, Oct 24, 2016 at 9:59 PM, Ronald S. Bultje 
> wrote:
> > Good idea to reference Hendrik Gramner here, who keeps insisting we get
> rid
> > of all MMX code in ffmpeg (at least as an option) for future Intel CPUs
> in
> > which MMX will be deprecated.
>
> Replacing MMX with SSE2 is indeed the most "proper" fix in my opinion,
> but it's a fair amount of work and not done in an evening.
>
> The fact that a lot of assembly lacks unit tests is certainly not
> helping in that regard.
>
> Some MMX instructions are slower than the equivalent SSE2 code on
> Skylake. Intel hasn't officially commented on (as far as I know at
> least) if we should expect this trend to continue, but they certainly
> seem to treat MMX as legacy.
>
> I doubt they would completely remove support for it though, backwards
> compatibility is a big selling-point for x86.


Well, it gives us another way of fixing this issue (on x86-64 only): have
sse2 implementations for all code that has a mmx (register) path right now.

Given the complexity of this issue, it's worth considering that fix along
with the other possibilities...

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread Henrik Gramner
On Mon, Oct 24, 2016 at 9:59 PM, Ronald S. Bultje  wrote:
> Good idea to reference Hendrik Gramner here, who keeps insisting we get rid
> of all MMX code in ffmpeg (at least as an option) for future Intel CPUs in
> which MMX will be deprecated.

Replacing MMX with SSE2 is indeed the most "proper" fix in my opinion,
but it's a fair amount of work and not done in an evening.

The fact that a lot of assembly lacks unit tests is certainly not
helping in that regard.

Some MMX instructions are slower than the equivalent SSE2 code on
Skylake. Intel hasn't officially commented on (as far as I know at
least) if we should expect this trend to continue, but they certainly
seem to treat MMX as legacy.

I doubt they would completely remove support for it though, backwards
compatibility is a big selling-point for x86.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] vf_colorspace: don't spam console with warnings if range is unspecified.

2016-10-24 Thread Ronald S. Bultje
Hi,

On Sat, Oct 22, 2016 at 10:02 PM, Ronald S. Bultje 
wrote:

> Hi,
>
> On Sat, Oct 22, 2016 at 12:19 PM, Moritz Barsnick 
> wrote:
>
>> On Sat, Oct 22, 2016 at 08:43:26 -0400, Ronald S. Bultje wrote:
>> > Is there documentation on what coverity expects?
>>
>> I found this:
>> https://lost-contact.mit.edu/afs/cs.stanford.edu/pkg/prevent
>> -4.3.1/i386_linux26/opt/prevent-linux-4.3.1/cgi-bin/
>> doc/checker_ref.html#c_checker_MISSING_BREAK
>
>
> I was hoping for documentation on what it expects, not what can be changed
> about it :) Anyway, I'll change it, I don't really care.
>

Pushed with that modification.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/4] lavfi: split frame_count between input and output.

2016-10-24 Thread Paul B Mahol
On 10/23/16, Nicolas George  wrote:
> AVFilterLink.frame_count is supposed to count the number of frames
> that were passed on the link, but with min_samples, that number is
> not always the same for the source and destination filters.
> With the addition of a FIFO on the link, the difference will become
> more significant.
>
> Split the variable in two: frame_count_in counts the number of
> frames that entered the link, frame_count_out counts the number
> of frames that were sent to the destination filter.
>
> Signed-off-by: Nicolas George 
> ---
>  libavfilter/af_ashowinfo.c   |  2 +-
>  libavfilter/af_volume.c  |  2 +-
>  libavfilter/asrc_sine.c  |  2 +-
>  libavfilter/avf_showfreqs.c  |  4 ++--
>  libavfilter/avfilter.c   |  5 +++--
>  libavfilter/avfilter.h   |  2 +-
>  libavfilter/f_loop.c |  2 +-
>  libavfilter/f_metadata.c |  4 ++--
>  libavfilter/f_select.c   |  2 +-
>  libavfilter/f_streamselect.c |  2 +-
>  libavfilter/vf_bbox.c|  2 +-
>  libavfilter/vf_blackdetect.c |  2 +-
>  libavfilter/vf_blend.c   |  2 +-
>  libavfilter/vf_crop.c|  2 +-
>  libavfilter/vf_decimate.c|  2 +-
>  libavfilter/vf_detelecine.c  |  2 +-
>  libavfilter/vf_drawtext.c|  4 ++--
>  libavfilter/vf_eq.c  |  2 +-
>  libavfilter/vf_fade.c|  8 
>  libavfilter/vf_fieldhint.c   | 14 +++---
>  libavfilter/vf_fieldmatch.c  |  6 +++---
>  libavfilter/vf_framestep.c   |  2 +-
>  libavfilter/vf_geq.c |  2 +-
>  libavfilter/vf_hue.c |  2 +-
>  libavfilter/vf_overlay.c |  2 +-
>  libavfilter/vf_paletteuse.c  |  2 +-
>  libavfilter/vf_perspective.c |  4 ++--
>  libavfilter/vf_rotate.c  |  2 +-
>  libavfilter/vf_showinfo.c|  2 +-
>  libavfilter/vf_swaprect.c|  2 +-
>  libavfilter/vf_telecine.c|  2 +-
>  libavfilter/vf_tinterlace.c  |  4 ++--
>  libavfilter/vf_vignette.c|  2 +-
>  libavfilter/vf_zoompan.c |  6 +++---
>  libavfilter/vsrc_mptestsrc.c |  2 +-
>  35 files changed, 55 insertions(+), 54 deletions(-)
>

Idea sounds sane, i havent chacked each change if it is correct.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] vp9: change order of operations in adapt_prob().

2016-10-24 Thread Ronald S. Bultje
Hi,

On Fri, Oct 14, 2016 at 2:09 PM, James Zern  wrote:

> Ronald,
>
> On Fri, Oct 14, 2016 at 10:01 AM, Ronald S. Bultje 
> wrote:
> > This is intended to workaround bug "665 Integer Divide Instruction May
> > Cause Unpredictable Behavior" on some early AMD CPUs, which causes a
> > div-by-zero in this codepath, such as reported in Mozilla bug #1293996.
> >
> > Note that this isn't guaranteed to fix the bug, since a compiler is free
> > to reorder instructions that don't depend on each other. However, it
> > appears to fix the bug in Firefox, and a similar patch was applied to
> > libvpx also (see Chrome bug #599899).
> >
>
> I recently made a few additional changes as this regressed in chrome
> [1][2], but just like this change there's no guarantee it won't occur
> again.
>
> [1] https://chromium.googlesource.com/webm/libvpx/+/
> 8b4210940ce4183d4cfded42c323612c0c6d1688
> [2] https://chromium.googlesource.com/webm/libvpx/+/
> 82ea74223771793628dbd812c2fd50afcfb8183a
>
> > ---
> >  libavcodec/vp9.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
>
> lgtm


(Firefox beta crash reports confirmed that the bug is fixed.)

Pushed.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] vf_colorspace: Add support for iec61966-2.1 (sRGB) transfer

2016-10-24 Thread Ronald S. Bultje
Hi,

On Thu, Oct 20, 2016 at 12:37 PM, Michael Niedermayer <
mich...@niedermayer.cc> wrote:

> On Tue, Oct 18, 2016 at 03:17:26PM -0400, Vittorio Giovara wrote:
> > Signed-off-by: Vittorio Giovara 
> > ---
> > Special thanks to Andrew Roland for helping me figure out these numbers.
> > Please keep me in CC.
> > Vittorio
> >
> >  libavfilter/vf_colorspace.c | 3 +++
> >  1 file changed, 3 insertions(+)
>
> LGTM


Pushed.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread wm4
On Mon, 24 Oct 2016 21:53:22 +0200
Henrik Gramner  wrote:

> On Mon, Oct 24, 2016 at 9:34 PM, wm4  wrote:
> > a ASM function must, according to the calling convention, reset the
> > MMX state when returning.
> >
> > What FFmpeg does here was misdesigned from the very start.  
> 
> The decision to issue emms manually instead of after every MMX
> function was a deliberate decision. I'd hardly call it "misdesigned"
> to make SIMD code twice as fast at the cost of technically abusing the
> ABI, which has worked flawlessly for years until very recently.

It's still clearly not a good idea because it violates the ABI directly.

OK let's get back to the "practical" argument that we have not much of
a choice but to violate the ABI.

Then at least the "weaker" assumption, that emms should be called before
calling (or returning to) functions that don't explicitly support MMX,
should be followed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread Ronald S. Bultje
Hi,

On Mon, Oct 24, 2016 at 3:34 PM, wm4  wrote:

> On Mon, 24 Oct 2016 21:19:46 +0200
> Andreas Cadhalpun  wrote:
>
> > On 24.10.2016 16:14, Ronald S. Bultje wrote:
> > > On Mon, Oct 24, 2016 at 8:47 AM, wm4  wrote:
> > >> On Mon, 24 Oct 2016 07:54:47 -0400
> > >> "Ronald S. Bultje"  wrote:
> > >>> On Mon, Oct 24, 2016 at 3:36 AM, wm4  wrote:
> >  I was under the impression that it is UB to have the FPU in MMX
> state
> >  at any time while in C, not just while e.g. calling the stdlib.
> Maybe I
> >  got that wrong (how would MMX intrinsics even work?) - can anyone
> shed
> >  light on the exact requirements? (Possibly again, sorry.)
> > >>>
> > >>> I'm under the impression that it's part of the calling convention.
> That
> > >> is,
> > >>> any code anywhere (including mmx intrinsics, indeed) can - when
> called -
> > >>> expect the state to be cleared by the caller, just like you'd expect
> > >>> eax/edx to be caller-save (whereas esi/edi are callee-save).
> > >>>
> > >>> However, if you never call external code (including intrinsics), you
> can
> > >>> ignore this, just as you can ignore / create your own calling
> > >>> convention (remember fastcall etc.?). However, when calling any
> external
> > >>> code, this could (in theory) crash; it's just that right now it only
> > >>> crashes with musl when calling malloc/free. So basically, ffmpeg has
> its
> > >>> own calling convention, and manually calling emms_c() fixes "ffmpeg"
> > >>> calling convention to be compatible with "standard" calling
> > >> convention...?
> > >>
> > >> It can't really be a calling convention unless the compiler is aware
> of
> > >> it?
> >
> > It is defined as part of the System V Application Binary Interface [1]:
> > "The CPU shall be in x87 mode upon entry to a function. Therefore, every
> > function that uses the MMX registers is required to issue an emms or
> femms
> > instruction after using MMX registers, before returning or calling
> another
> > function."
>
> I mean FFmpeg can't make up its own calling convention without the
> compiler's knowledge.
>
> But thanks for reminding me about this but of the sysv ABI. The
> paragraph you quoted is actually very clear about the requirements. It
> means FFmpeg can barely do anything and remain standard compliant: a
> ASM function must, according to the calling convention, reset the MMX
> state when returning.
>
> What FFmpeg does here was misdesigned from the very start.


Good idea to reference Hendrik Gramner here, who keeps insisting we get rid
of all MMX code in ffmpeg (at least as an option) for future Intel CPUs in
which MMX will be deprecated.

Food for thought, indeed.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread Henrik Gramner
On Mon, Oct 24, 2016 at 9:34 PM, wm4  wrote:
> a ASM function must, according to the calling convention, reset the
> MMX state when returning.
>
> What FFmpeg does here was misdesigned from the very start.

The decision to issue emms manually instead of after every MMX
function was a deliberate decision. I'd hardly call it "misdesigned"
to make SIMD code twice as fast at the cost of technically abusing the
ABI, which has worked flawlessly for years until very recently.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread Andreas Cadhalpun
On 24.10.2016 21:34, wm4 wrote:
> On Mon, 24 Oct 2016 21:19:46 +0200
> Andreas Cadhalpun  wrote:
>> On 24.10.2016 16:14, Ronald S. Bultje wrote:
>>> On Mon, Oct 24, 2016 at 8:47 AM, wm4  wrote:  
 The next safest assumption is that it's fine as long as we explicitly
 don't use floating point or call external functions that aren't
 MMX-aware. This would mean calling any libc functions or user callbacks
 (including indirectly through e.g. av_log) is not fine.  
>>
>> This is probably OK in practice and likely has a significant performance
>> benefit.
> 
> Seems like it.
> 
> The compiler still could generate code using the FPU state at any
> point, though, unless maybe there is an inline asm block. No function
> can put the FPU into MMX mode, because any MMX using function must have
> called emms before returning. Consequently, only compiler-known
> intrinsics or opaque inline asm block could clobber the FPU state. The
> latter because the compiler doesn't inspect asm blocks.

I agree. However, I think that this is currently only a theoretical
issue. At least I'm not aware of this causing problems with a real compiler.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: fix avcc creation for h264 streams missing extradata

2016-10-24 Thread wm4
On Mon, 24 Oct 2016 12:18:07 -0700
Aman Gupta  wrote:

> On Sun, Oct 23, 2016 at 11:20 PM, Aman Gupta  wrote:
> 
> >
> >
> > On Sunday, October 23, 2016, Richard Kern  wrote:
> >  
> >>  
> >> > On Oct 19, 2016, at 8:45 PM, Aman Gupta  wrote:
> >> >
> >> > From: Aman Gupta 
> >> >
> >> > ff_videotoolbox_avcc_extradata_create() was never being called if
> >> > avctx->extradata_size==0, even though the function does not need or use
> >> > the avctx->extradata.  
> >>
> >> Could this be a bug in another part of the code? It seems like extradata
> >> should be set.  
> >
> >
> > Yes it definitely could be. I can verify if extradata is present on macOS
> > where the same sample and ffmpeg version worked.
> >  
> 
> Looks like missing extradata has nothing to do with the platform, but
> rather only happens when you skip calling avformat_find_stream_info().
> 
> I am skipping the find_stream_info() call to reduce time spent probing and
> begin playback more quickly, since the initial avformat_open_input() is
> enough to discover available streams on mpegts containers.
> 
> That means this issue is not as widespread as I initially thought, but I
> still think it's an optimization worth making.

That makes sense. I think a decoder shouldn't really rely on this
pseudo extradata that libavformat constructs.

That this code is run only with extradata_size set makes no sense at
least in the h264 code and was an oversight.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread Andreas Cadhalpun
On 24.10.2016 16:14, Ronald S. Bultje wrote:
> On Mon, Oct 24, 2016 at 8:47 AM, wm4  wrote:
>> On Mon, 24 Oct 2016 07:54:47 -0400
>> "Ronald S. Bultje"  wrote:
>>> On Mon, Oct 24, 2016 at 3:36 AM, wm4  wrote:
 I was under the impression that it is UB to have the FPU in MMX state
 at any time while in C, not just while e.g. calling the stdlib. Maybe I
 got that wrong (how would MMX intrinsics even work?) - can anyone shed
 light on the exact requirements? (Possibly again, sorry.)
>>>
>>> I'm under the impression that it's part of the calling convention. That
>> is,
>>> any code anywhere (including mmx intrinsics, indeed) can - when called -
>>> expect the state to be cleared by the caller, just like you'd expect
>>> eax/edx to be caller-save (whereas esi/edi are callee-save).
>>>
>>> However, if you never call external code (including intrinsics), you can
>>> ignore this, just as you can ignore / create your own calling
>>> convention (remember fastcall etc.?). However, when calling any external
>>> code, this could (in theory) crash; it's just that right now it only
>>> crashes with musl when calling malloc/free. So basically, ffmpeg has its
>>> own calling convention, and manually calling emms_c() fixes "ffmpeg"
>>> calling convention to be compatible with "standard" calling
>> convention...?
>>
>> It can't really be a calling convention unless the compiler is aware of
>> it?

It is defined as part of the System V Application Binary Interface [1]:
"The CPU shall be in x87 mode upon entry to a function. Therefore, every
function that uses the MMX registers is required to issue an emms or femms
instruction after using MMX registers, before returning or calling another
function."

>> We're doing things behind the compiler's back here. The safest
>> assumption would be that leaving the FPU state invalid while in C is
>> not allowed, period.

That's what the standard says.

>> The next safest assumption is that it's fine as long as we explicitly
>> don't use floating point or call external functions that aren't
>> MMX-aware. This would mean calling any libc functions or user callbacks
>> (including indirectly through e.g. av_log) is not fine.

This is probably OK in practice and likely has a significant performance
benefit.

Best regards,
Andreas


1: 
https://software.intel.com/sites/default/files/article/402129/mpx-linux64-abi.pdf

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: fix avcc creation for h264 streams missing extradata

2016-10-24 Thread Aman Gupta
On Sun, Oct 23, 2016 at 11:20 PM, Aman Gupta  wrote:

>
>
> On Sunday, October 23, 2016, Richard Kern  wrote:
>
>>
>> > On Oct 19, 2016, at 8:45 PM, Aman Gupta  wrote:
>> >
>> > From: Aman Gupta 
>> >
>> > ff_videotoolbox_avcc_extradata_create() was never being called if
>> > avctx->extradata_size==0, even though the function does not need or use
>> > the avctx->extradata.
>>
>> Could this be a bug in another part of the code? It seems like extradata
>> should be set.
>
>
> Yes it definitely could be. I can verify if extradata is present on macOS
> where the same sample and ffmpeg version worked.
>

Looks like missing extradata has nothing to do with the platform, but
rather only happens when you skip calling avformat_find_stream_info().

I am skipping the find_stream_info() call to reduce time spent probing and
begin playback more quickly, since the initial avformat_open_input() is
enough to discover available streams on mpegts containers.

That means this issue is not as widespread as I initially thought, but I
still think it's an optimization worth making.


>
>
>>
>> >
>> > This manifested itself only on h264 streams in specific containers, and
>> > only on iOS. I guess the macOS version of VideoToolbox is more
>> > forgiving, atleast on my specific combination of OS version and
>> hardware.
>>
>> Which container has this issue?
>
>
> I saw it with an mpegts file, but only on iOS.
>
>
>>
>> >
>> > I also added an error log message when VTDecompressionSessionCreate()
>> > fails, to help the next poor soul who runs into a bug in this area of
>> the
>> > code. The native OSStatus error codes are much easier to google than
>> > their AVERROR counterparts (especially in this case, with
>> AVERROR_UNKNOWN).
>> > ---
>> > libavcodec/videotoolbox.c | 7 +--
>> > 1 file changed, 5 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
>> > index 1288aa5..b21eccb 100644
>> > --- a/libavcodec/videotoolbox.c
>> > +++ b/libavcodec/videotoolbox.c
>> > @@ -413,7 +413,7 @@ static CFDictionaryRef
>> videotoolbox_decoder_config_create(CMVideoCodecType codec
>> >  kVTVideoDecoderSpecification_R
>> equireHardwareAcceleratedVideoDecoder,
>> >  kCFBooleanTrue);
>> >
>> > -if (avctx->extradata_size) {
>> > +if (avctx->extradata_size || codec_type == kCMVideoCodecType_H264)
>> {
>>
>> This is somewhat confusing. The extradata_size check is only needed for
>> videotoolbox_esds_extradata_create(). It should check the sps and pps
>> sizes are valid before creating the avcc atom.
>
>
> I can remove this outer if statement altogether, and add a check either
> around or inside the esds_create if that makes more sense.
>

Let me know if you'd like me to submit the alternative version of this
patch.


>
>
>>
>> > CFMutableDictionaryRef avc_info;
>> > CFDataRef data = NULL;
>> >
>> > @@ -572,13 +572,16 @@ static int videotoolbox_default_init(AVCodecContext
>> *avctx)
>> > if (buf_attr)
>> > CFRelease(buf_attr);
>> >
>> > +if (status != 0)
>> > +av_log(avctx, AV_LOG_ERROR, "Error creating videotoolbox
>> decompression session: %d\n", status);
>> > +
>> > switch (status) {
>> > case kVTVideoDecoderNotAvailableNowErr:
>> > case kVTVideoDecoderUnsupportedDataFormatErr:
>> > return AVERROR(ENOSYS);
>> > case kVTVideoDecoderMalfunctionErr:
>> > return AVERROR(EINVAL);
>> > -case kVTVideoDecoderBadDataErr :
>> > +case kVTVideoDecoderBadDataErr:
>> > return AVERROR_INVALIDDATA;
>> > case 0:
>> > return 0;
>> > --
>> > 2.8.1
>> >
>> > ___
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread Andreas Cadhalpun
On 23.10.2016 20:18, Carl Eugen Hoyos wrote:
> 2016-10-23 12:14 GMT+02:00 Andreas Cadhalpun 
> :
> 
>> I also don't like adding emms_c in various places, but I don't see a
>> realistic alternative
> 
> (+1!)
> 
>> other than not fixing the problem. And I think that would be worse
>> than this patch set.
> 
> I don't (strongly) disagree but could you elaborate on why this would
> be such a bad alternative?

For one thing FFmpeg shouldn't rely on undefined behavior (a theoretical
issue) and for another thing it should work with musl libc (a practical
problem). I think not fixing the latter is bad for our users.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 07/11] lavf/movenc+dashenc: add automatic bitstream filtering

2016-10-24 Thread Michael Niedermayer
On Fri, Sep 09, 2016 at 11:37:21PM -0500, Rodger Combs wrote:
> This is disabled by default when the empty_moov flag is enabled
> ---
>  libavformat/dashenc.c |  43 +++-
>  libavformat/movenc.c  | 107 
> +++---
>  2 files changed, 124 insertions(+), 26 deletions(-)
[...]

> +static int mov_write_header(AVFormatContext *s)
> +{
> +AVIOContext *pb = s->pb;
> +MOVMuxContext *mov = s->priv_data;
> +AVDictionaryEntry *t, *global_tcr = av_dict_get(s->metadata, "timecode", 
> NULL, 0);
> +int i, ret, hint_track = 0, tmcd_track = 0, nb_tracks = s->nb_streams;
> +
> +if (mov->mode & (MODE_MP4|MODE_MOV|MODE_IPOD) && s->nb_chapters)
> +nb_tracks++;
> +
> +if (mov->flags & FF_MOV_FLAG_RTP_HINT) {
> +/* Add hint tracks for each audio and video stream */
> +hint_track = nb_tracks;
> +for (i = 0; i < s->nb_streams; i++) {
> +AVStream *st = s->streams[i];
> +if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO ||
> +st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
> +nb_tracks++;
> +}
> +}
> +}
> +
> +if (mov->mode == MODE_MOV || mov->mode == MODE_MP4)
> +tmcd_track = nb_tracks;

tmcd_track and hint_track are set but unused

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.

2016-10-24 Thread Michael Niedermayer
On Mon, Oct 24, 2016 at 02:36:23PM +0200, wm4 wrote:
> On Mon, 24 Oct 2016 13:24:38 +0200
> Michael Niedermayer  wrote:
> 
> > On Mon, Oct 24, 2016 at 09:38:00AM +0200, wm4 wrote:
> > > On Sun, 23 Oct 2016 05:37:25 +0200
> > > Michael Niedermayer  wrote:
> > >   
> > > > Signed-off-by: Michael Niedermayer 
> > > > ---
> > > >  libavutil/x86/emms.h | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> > > > index 6fda6e2..42c18e2 100644
> > > > --- a/libavutil/x86/emms.h
> > > > +++ b/libavutil/x86/emms.h
> > > > @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> > > >   * Empty mmx state.
> > > >   * this must be called between any dsp function and float/double code.
> > > >   * for example sin(); dsp->idct_put(); emms_c(); cos()
> > > > + * Note, *alloc() and *free() also use float code in some libc 
> > > > implementations
> > > > + * thus this also applies to them or any function using them.
> > > >   */
> > > >  static av_always_inline void emms_c(void)
> > > >  {  
> > > 
> > > Overly specific and useless information. It's an implementation detail  
> > 
> > If we place emms_c() so as to ensure that the FPU state is clean
> > before all calls to *alloc() and *free() (as is done in the posted
> > patchset) then we need to document this
> > so others working on the code are aware of it and wont mistakly break
> > it.
> > 
> > If/when a different or more complete solution is implemented then this
> > note needs to be adjusted accordingly.
> > 
> > [...]
> 
> That fixes only the musl-specific issue. It could happen any time again
> with a set of different callers.

yes


> Unless you want to put special effort
> into fixing operation with musl (and keeping it working) it won't help
> much to fix the correctness issues.

i did want to do that, but ive a headache atm and the discussion
about emms yesterday makes me not want to touch emms anymore


> 
> If anything, it should probably say "all external" calls or such.

I see this as the long term goal, yes i agree it should be
be documented if theres consensus about it

The way i imagined it was to fix the practically relevant issues and
anything else thats easy or attached (like x86-64 for what is the same
on x86-32) before 3.2 and have 3.2 work with musl on all archs
then aim at fixing the rest over the following months unless some
roadblocks like performance issues are hit.

This incremental approuch has been hit with quite some opposition so
ill leave this to whoever likes to work on this

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread Ronald S. Bultje
Hi,

On Mon, Oct 24, 2016 at 8:47 AM, wm4  wrote:

> On Mon, 24 Oct 2016 07:54:47 -0400
> "Ronald S. Bultje"  wrote:
>
> > Hi,
> >
> > On Mon, Oct 24, 2016 at 3:36 AM, wm4  wrote:
> >
> > > On Sun, 23 Oct 2016 13:02:01 -0400
> > > "Ronald S. Bultje"  wrote:
> > >
> > > > Hi,
> > > >
> > > > On Sat, Oct 22, 2016 at 11:36 PM, Michael Niedermayer <
> > > > mich...@niedermayer.cc> wrote:
> > > >
> > > > > On Sat, Oct 22, 2016 at 10:10:01PM -0400, Ronald S. Bultje wrote:
> > > > > > Hi,
> > > > > >
> > > > > > general comment about all other dec patches.
> > > > > >
> > > > > > On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer
> > > > >  > > > > > > wrote:
> > > > > >
> > > > > > > Signed-off-by: Michael Niedermayer 
> > > > > > > ---
> > > > > > >  libavcodec/svq1dec.c | 2 ++
> > > > > > >  1 file changed, 2 insertions(+)
> > > > > > >
> > > > > > > diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c
> > > > > > > index 2b72e08..0fe222e 100644
> > > > > > > --- a/libavcodec/svq1dec.c
> > > > > > > +++ b/libavcodec/svq1dec.c
> > > > > > > @@ -744,6 +744,7 @@ static int svq1_decode_frame(AVCodecConte
> xt
> > > > > *avctx,
> > > > > > > void *data,
> > > > > > >  }
> > > > > > >  }
> > > > > > >  }
> > > > > > > +emms_c();
> > > > > >
> > > > > >
> > > > > > This is hideous, you're sprinkling emms_c in various places to
> make
> > > some
> > > > > > stupid test pass. The test is morbidly stupid and there is no
> general
> > > > > > consensus on patterns to be followed as for where to place
> emms_c.
> > > > > Someone
> > > > > > who doesn't know any better will litter each new decoder with
> 10-20
> > > calls
> > > > > > to emms_c just because he found that other decoders do it in
> > > > > undocumented,
> > > > > > unexplained and unclear locations also.
> > > > > >
> > > > > > If you want this to be a "thing", you need to design and document
> > > > > carefully
> > > > > > where emms_c is necessary. Then come up with some system that
> makes
> > > this
> > > > > > work by itself.
> > > > >
> > > > > Your mail sounds quite aggressive to me, did i say something to
> anger
> > > > > you ? It was certainly not intended
> > > > >
> > > > > About this patchset
> > > > > FFmpeg is broken ATM (both in theory and practice with some libcs),
> > > > > the way MMX code is used is not correct, emms_c()
> > > > > calls are missing and required. The obvious way to fix that
> > > > > (in practice) is adding emms_c() calls where they are missing.
> > > > > I can document why each call is needed, if thats wanted?
> > > >
> > > >
> > > > Your representation of facts is strange, to say the least. Let's
> explore
> > > > two related claims:
> > > >
> > > > - (A) ffmpeg is broken in practice when calling musl malloc/free
> > > functions
> > > > after calling MMX SIMD functions on x86-32 (which crashes).
> > > > - (B) ffmpeg is broken in theory because we don't clear FPU state (as
> > > > required) at the end of MMX SIMD functions.
> > >
> > > I was under the impression that it is UB to have the FPU in MMX state
> > > at any time while in C, not just while e.g. calling the stdlib. Maybe I
> > > got that wrong (how would MMX intrinsics even work?) - can anyone shed
> > > light on the exact requirements? (Possibly again, sorry.)
> >
> >
> > I'm under the impression that it's part of the calling convention. That
> is,
> > any code anywhere (including mmx intrinsics, indeed) can - when called -
> > expect the state to be cleared by the caller, just like you'd expect
> > eax/edx to be caller-save (whereas esi/edi are callee-save).
> >
> > However, if you never call external code (including intrinsics), you can
> > ignore this, just as you can ignore / create your own calling
> > convention (remember fastcall etc.?). However, when calling any external
> > code, this could (in theory) crash; it's just that right now it only
> > crashes with musl when calling malloc/free. So basically, ffmpeg has its
> > own calling convention, and manually calling emms_c() fixes "ffmpeg"
> > calling convention to be compatible with "standard" calling
> convention...?
>
> It can't really be a calling convention unless the compiler is aware of
> it?
>
> We're doing things behind the compiler's back here. The safest
> assumption would be that leaving the FPU state invalid while in C is
> not allowed, period.
>
> The next safest assumption is that it's fine as long as we explicitly
> don't use floating point or call external functions that aren't
> MMX-aware. This would mean calling any libc functions or user callbacks
> (including indirectly through e.g. av_log) is not fine.
>

Yes.

And while I don't agree with it, I feel we should also mention Carl Eugen's
alternative proposal, which is to document that ffmpeg doesn't formally
adhere to this calling convention requirement [1].

Of course 

Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread wm4
On Mon, 24 Oct 2016 07:54:47 -0400
"Ronald S. Bultje"  wrote:

> Hi,
> 
> On Mon, Oct 24, 2016 at 3:36 AM, wm4  wrote:
> 
> > On Sun, 23 Oct 2016 13:02:01 -0400
> > "Ronald S. Bultje"  wrote:
> >  
> > > Hi,
> > >
> > > On Sat, Oct 22, 2016 at 11:36 PM, Michael Niedermayer <  
> > > mich...@niedermayer.cc> wrote:  
> > >  
> > > > On Sat, Oct 22, 2016 at 10:10:01PM -0400, Ronald S. Bultje wrote:  
> > > > > Hi,
> > > > >
> > > > > general comment about all other dec patches.
> > > > >
> > > > > On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer  
> > > >  > > > > > wrote:  
> > > > >  
> > > > > > Signed-off-by: Michael Niedermayer 
> > > > > > ---
> > > > > >  libavcodec/svq1dec.c | 2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c
> > > > > > index 2b72e08..0fe222e 100644
> > > > > > --- a/libavcodec/svq1dec.c
> > > > > > +++ b/libavcodec/svq1dec.c
> > > > > > @@ -744,6 +744,7 @@ static int svq1_decode_frame(AVCodecContext  
> > > > *avctx,  
> > > > > > void *data,
> > > > > >  }
> > > > > >  }
> > > > > >  }
> > > > > > +emms_c();  
> > > > >
> > > > >
> > > > > This is hideous, you're sprinkling emms_c in various places to make  
> > some  
> > > > > stupid test pass. The test is morbidly stupid and there is no general
> > > > > consensus on patterns to be followed as for where to place emms_c.  
> > > > Someone  
> > > > > who doesn't know any better will litter each new decoder with 10-20  
> > calls  
> > > > > to emms_c just because he found that other decoders do it in  
> > > > undocumented,  
> > > > > unexplained and unclear locations also.
> > > > >
> > > > > If you want this to be a "thing", you need to design and document  
> > > > carefully  
> > > > > where emms_c is necessary. Then come up with some system that makes  
> > this  
> > > > > work by itself.  
> > > >
> > > > Your mail sounds quite aggressive to me, did i say something to anger
> > > > you ? It was certainly not intended
> > > >
> > > > About this patchset
> > > > FFmpeg is broken ATM (both in theory and practice with some libcs),
> > > > the way MMX code is used is not correct, emms_c()
> > > > calls are missing and required. The obvious way to fix that
> > > > (in practice) is adding emms_c() calls where they are missing.
> > > > I can document why each call is needed, if thats wanted?  
> > >
> > >
> > > Your representation of facts is strange, to say the least. Let's explore
> > > two related claims:
> > >
> > > - (A) ffmpeg is broken in practice when calling musl malloc/free  
> > functions  
> > > after calling MMX SIMD functions on x86-32 (which crashes).
> > > - (B) ffmpeg is broken in theory because we don't clear FPU state (as
> > > required) at the end of MMX SIMD functions.  
> >
> > I was under the impression that it is UB to have the FPU in MMX state
> > at any time while in C, not just while e.g. calling the stdlib. Maybe I
> > got that wrong (how would MMX intrinsics even work?) - can anyone shed
> > light on the exact requirements? (Possibly again, sorry.)  
> 
> 
> I'm under the impression that it's part of the calling convention. That is,
> any code anywhere (including mmx intrinsics, indeed) can - when called -
> expect the state to be cleared by the caller, just like you'd expect
> eax/edx to be caller-save (whereas esi/edi are callee-save).
> 
> However, if you never call external code (including intrinsics), you can
> ignore this, just as you can ignore / create your own calling
> convention (remember fastcall etc.?). However, when calling any external
> code, this could (in theory) crash; it's just that right now it only
> crashes with musl when calling malloc/free. So basically, ffmpeg has its
> own calling convention, and manually calling emms_c() fixes "ffmpeg"
> calling convention to be compatible with "standard" calling convention...?

It can't really be a calling convention unless the compiler is aware of
it?

We're doing things behind the compiler's back here. The safest
assumption would be that leaving the FPU state invalid while in C is
not allowed, period.

The next safest assumption is that it's fine as long as we explicitly
don't use floating point or call external functions that aren't
MMX-aware. This would mean calling any libc functions or user callbacks
(including indirectly through e.g. av_log) is not fine.

Of course this is not as easy to verify as adding a hack-assert to
av_malloc/av_free.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.

2016-10-24 Thread wm4
On Mon, 24 Oct 2016 13:24:38 +0200
Michael Niedermayer  wrote:

> On Mon, Oct 24, 2016 at 09:38:00AM +0200, wm4 wrote:
> > On Sun, 23 Oct 2016 05:37:25 +0200
> > Michael Niedermayer  wrote:
> >   
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavutil/x86/emms.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> > > index 6fda6e2..42c18e2 100644
> > > --- a/libavutil/x86/emms.h
> > > +++ b/libavutil/x86/emms.h
> > > @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> > >   * Empty mmx state.
> > >   * this must be called between any dsp function and float/double code.
> > >   * for example sin(); dsp->idct_put(); emms_c(); cos()
> > > + * Note, *alloc() and *free() also use float code in some libc 
> > > implementations
> > > + * thus this also applies to them or any function using them.
> > >   */
> > >  static av_always_inline void emms_c(void)
> > >  {  
> > 
> > Overly specific and useless information. It's an implementation detail  
> 
> If we place emms_c() so as to ensure that the FPU state is clean
> before all calls to *alloc() and *free() (as is done in the posted
> patchset) then we need to document this
> so others working on the code are aware of it and wont mistakly break
> it.
> 
> If/when a different or more complete solution is implemented then this
> note needs to be adjusted accordingly.
> 
> [...]

That fixes only the musl-specific issue. It could happen any time again
with a set of different callers. Unless you want to put special effort
into fixing operation with musl (and keeping it working) it won't help
much to fix the correctness issues.

If anything, it should probably say "all external" calls or such.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread Clément Bœsch
On Mon, Oct 24, 2016 at 01:31:10PM +0200, u-9...@aetey.se wrote:
> On Mon, Oct 24, 2016 at 12:18:23PM +0200, Clément Bœsch wrote:
> > Today, in practice, we have to support all kind of mix of subtitles markup
> > in various formats because of this. It would be great not to make things
> > worst :)
> 
> Let's see.
> 
> Looking at kateenc code I believe that the intention was to keep
> Kate "compatible with SRT markup" whatever it means.
> 
> The encoder does not interpret/recode markup between srt and kate. What
> it does is to handle the metainformation about the presence of markup
> (which presence or absence is explicit in Kate).
> 
> So we will quite certainly do the users a favor by _not_ cleaning the
> possibly present markup tags, when converting from kate to srt.
> 
> Does this sound good enough?

I could replace srt with any other subtitles format. Whether it's Kate or
SubRip markup, I don't want it in ASS or MicroDVD.

If the Kate markup is the same as SRT, then you should output SubRip
packets in the OGG demuxer, and you do not need to write a Kate decoder.
Otherwise, you need to do just like the SRT decoder: handle the markup
(that is, decode it as the internal representation), or just strip it for
a start.

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.

2016-10-24 Thread Michael Niedermayer
On Mon, Oct 24, 2016 at 09:38:00AM +0200, wm4 wrote:
> On Sun, 23 Oct 2016 05:37:25 +0200
> Michael Niedermayer  wrote:
> 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavutil/x86/emms.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> > index 6fda6e2..42c18e2 100644
> > --- a/libavutil/x86/emms.h
> > +++ b/libavutil/x86/emms.h
> > @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> >   * Empty mmx state.
> >   * this must be called between any dsp function and float/double code.
> >   * for example sin(); dsp->idct_put(); emms_c(); cos()
> > + * Note, *alloc() and *free() also use float code in some libc 
> > implementations
> > + * thus this also applies to them or any function using them.
> >   */
> >  static av_always_inline void emms_c(void)
> >  {
> 
> Overly specific and useless information. It's an implementation detail

If we place emms_c() so as to ensure that the FPU state is clean
before all calls to *alloc() and *free() (as is done in the posted
patchset) then we need to document this
so others working on the code are aware of it and wont mistakly break
it.

If/when a different or more complete solution is implemented then this
note needs to be adjusted accordingly.

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread Clément Bœsch
On Mon, Oct 24, 2016 at 12:12:23PM +0200, u-9...@aetey.se wrote:
> On Mon, Oct 24, 2016 at 11:49:25AM +0200, Clément Bœsch wrote:
> > > It is bad if you don't strip the markup in the decoder.
> 
> > To expand on this: it is extremely worrisome that ffmpeg could be the
> > source of yet another wave of insane .srt files with kate markup in it
> > because someone decided to run ffmpeg -i in.ogg out.srt in order to
> > extract the subtitles and share them.
> > 
> > You absolutely want to strip all markup in the decoder for minimal
> > support, or FFmpeg will generate broken files, which is not tolerable.
> 
> I see and have to agree.
> 

Thanks.

> (even though I would not expect any remarkable harm to happen in practice)
> 

Today, in practice, we have to support all kind of mix of subtitles markup
in various formats because of this. It would be great not to make things
worst :)

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread u-9iep
On Mon, Oct 24, 2016 at 11:49:25AM +0200, Clément Bœsch wrote:
> > It is bad if you don't strip the markup in the decoder.

> To expand on this: it is extremely worrisome that ffmpeg could be the
> source of yet another wave of insane .srt files with kate markup in it
> because someone decided to run ffmpeg -i in.ogg out.srt in order to
> extract the subtitles and share them.
> 
> You absolutely want to strip all markup in the decoder for minimal
> support, or FFmpeg will generate broken files, which is not tolerable.

I see and have to agree.

(even though I would not expect any remarkable harm to happen in practice)

Regards,
Rune

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread Clément Bœsch
On Mon, Oct 24, 2016 at 11:58:41AM +0200, u-9...@aetey.se wrote:
[...]
> Could be added later if desired, either as an incremental patch or by
> introducing an optional dependency on libkate (which would open for full
> Kate functionality).

No, it's a blocking requirement (see my next mail), it needs to be
included in the initial support.

Regards,

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread u-9iep
On Mon, Oct 24, 2016 at 11:46:07AM +0200, Clément Bœsch wrote:
> On Mon, Oct 24, 2016 at 11:39:40AM +0200, u-9...@aetey.se wrote:
> > Yes, you are right, the patch just ignores the possible presence
> > of Kate markup (does not try to interpret it, nor removes).
> > This is probably not too bad, for the minimal support.
> 
> It is bad if you don't strip the markup in the decoder.

It shouldn't be hard (the kate_text_remove_markup() function in libkate
is just 49 lines) but was not included in the original patch.

Could be added later if desired, either as an incremental patch or by
introducing an optional dependency on libkate (which would open for full
Kate functionality).

Regards,
Rune

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread Clément Bœsch
On Mon, Oct 24, 2016 at 11:46:07AM +0200, Clément Bœsch wrote:
> On Mon, Oct 24, 2016 at 11:39:40AM +0200, u-9...@aetey.se wrote:
> > On Mon, Oct 24, 2016 at 11:09:43AM +0200, wm4 wrote:
> > > > Text subtitles in ogg kate are a straightforward way to put srt-like 
> > > > data
> > > 
> > > Note that srt supports simple HTML-like tags.
> > 
> > Yes, you are right, the patch just ignores the possible presence
> > of Kate markup (does not try to interpret it, nor removes).
> > This is probably not too bad, for the minimal support.
> > 
> 
> It is bad if you don't strip the markup in the decoder.
> 

To expand on this: it is extremely worrisome that ffmpeg could be the
source of yet another wave of insane .srt files with kate markup in it
because someone decided to run ffmpeg -i in.ogg out.srt in order to
extract the subtitles and share them.

You absolutely want to strip all markup in the decoder for minimal
support, or FFmpeg will generate broken files, which is not tolerable.

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread Clément Bœsch
On Mon, Oct 24, 2016 at 11:39:40AM +0200, u-9...@aetey.se wrote:
> On Mon, Oct 24, 2016 at 11:09:43AM +0200, wm4 wrote:
> > > Text subtitles in ogg kate are a straightforward way to put srt-like data
> > 
> > Note that srt supports simple HTML-like tags.
> 
> Yes, you are right, the patch just ignores the possible presence
> of Kate markup (does not try to interpret it, nor removes).
> This is probably not too bad, for the minimal support.
> 

It is bad if you don't strip the markup in the decoder.

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread Carl Eugen Hoyos
2016-10-24 10:52 GMT+02:00  :
> worth to mention:
>
> On Mon, Oct 24, 2016 at 10:25:44AM +0200, u-9...@aetey.se wrote:
>> > >https://trac.ffmpeg.org/ticket/3039
>
> The ticket refers to a sample with graphical subtitles. This is _not_
> being solved by the proposed patch.
>
> A sample with text subtitles can be seen e.g. at
>  
> http://people.xiph.org/~oggk/elephants_dream/elephantsdream-with-subtitles.ogg
> It is well decodable with the patch applied.

Thank you for explaining this!

> Text subtitles in ogg kate are a straightforward way to put srt-like data
> into ogg. Multiplexing kate into ogg is not addressed by the proposed
> patch, but is otherwise easily done by external tools. Such tools are
> not applicable as much during playback, that's why demuxing support
> is most important.

+1

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread u-9iep
On Mon, Oct 24, 2016 at 11:09:43AM +0200, wm4 wrote:
> > Text subtitles in ogg kate are a straightforward way to put srt-like data
> 
> Note that srt supports simple HTML-like tags.

Yes, you are right, the patch just ignores the possible presence
of Kate markup (does not try to interpret it, nor removes).
This is probably not too bad, for the minimal support.

Rune

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread wm4
On Mon, 24 Oct 2016 10:52:14 +0200
u-9...@aetey.se wrote:

> worth to mention:
> 
> On Mon, Oct 24, 2016 at 10:25:44AM +0200, u-9...@aetey.se wrote:
> > > >https://trac.ffmpeg.org/ticket/3039  
> 
> The ticket refers to a sample with graphical subtitles. This is _not_
> being solved by the proposed patch.
> 
> A sample with text subtitles can be seen e.g. at
>  
> http://people.xiph.org/~oggk/elephants_dream/elephantsdream-with-subtitles.ogg
> It is well decodable with the patch applied.
> 
> Text subtitles in ogg kate are a straightforward way to put srt-like data

Note that srt supports simple HTML-like tags.

> into ogg. Multiplexing kate into ogg is not addressed by the proposed
> patch, but is otherwise easily done by external tools. Such tools are
> not applicable as much during playback, that's why demuxing support
> is most important.
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread u-9iep
worth to mention:

On Mon, Oct 24, 2016 at 10:25:44AM +0200, u-9...@aetey.se wrote:
> > >https://trac.ffmpeg.org/ticket/3039

The ticket refers to a sample with graphical subtitles. This is _not_
being solved by the proposed patch.

A sample with text subtitles can be seen e.g. at
 http://people.xiph.org/~oggk/elephants_dream/elephantsdream-with-subtitles.ogg
It is well decodable with the patch applied.

Text subtitles in ogg kate are a straightforward way to put srt-like data
into ogg. Multiplexing kate into ogg is not addressed by the proposed
patch, but is otherwise easily done by external tools. Such tools are
not applicable as much during playback, that's why demuxing support
is most important.

Rune

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] add hds demuxer

2016-10-24 Thread Steven Liu
2016-10-24 15:22 GMT+08:00 wm4 :

> On Sat, 15 Oct 2016 15:52:48 +0200
> Nicolas George  wrote:
>
> > Le tridi 23 vendémiaire, an CCXXV, wm4 a écrit :
> > > XML is very complex
> >
> > This is the usual, and often only, argument raised in this kind of
> > situation. And unfortunately, I already addressed it in this very thread.
> >
> > XML is very complex, yes: processing instructions, entity definitions,
> DTSs
> > and schemas, namespaces, etc.
> >
> > Fortunately, we do not need that. Manifests only use the most basic XML
> > features: elements, attributes, text. Parsing that is easy.
> >
> > Regards,
> >
>
> Well, I thought the idea was already rejected by multiple people before.
>

But i think Nicolas George's idea is useful now, for example used in hds
mkv and some format depend on simple xml format.

> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread u-9iep
On Mon, Oct 24, 2016 at 10:00:38AM +0200, wm4 wrote:
> > The patch addresses the missing Kate subtitles support, reflected
> > in trac as
> >https://trac.ffmpeg.org/ticket/3039

> I don't quite get it. Doesn't this just demux kate subtitles as text
> without stripping whatever formatting tags kate might support? (I was
> under the impression that kate does more than just plain text.)

Kate can of course do much more than plain text, but this patch only
adds the recognition of ogg-kate-text, nothing else. That's what makes
it a low hanging fruit.

At the same time we get a remarkable improvement, compared to the complete
inability to handle subtitles in ogg.

Regards,
Rune

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] ogg kate text subtitles support (patch)

2016-10-24 Thread wm4
On Mon, 24 Oct 2016 09:31:25 +0200
u-9...@aetey.se wrote:

> Hello,
> 
> Given the practical constraints I can not thoroughly fulfill all the
> requirements for submitting a patch. I hope it can make it at least to
> the list archive, for possible future perusal by someone.
> 
> The patch addresses the missing Kate subtitles support, reflected
> in trac as
>https://trac.ffmpeg.org/ticket/3039
> 
> It is based on the patch present in the kate libraries as of 2011-10-12
> (https://git.xiph.org/?p=users/oggk/kate.git)
> 
> The attached version applies cleanly to ffmpeg-3.1.3 and does not introduce
> any dependencies.
> 
> It makes ffmpeg and the programs using ffmpeg, like mplayer,
> properly recognize and decode ogg kate text subtitles.
> 
> Thanks for the excellent ffmpeg software.
> 
> This is my attempt to make a small contribution, for a minor but
> regrettably missing and inexpensive feature.
> 
> Regards,
> Rune

I don't quite get it. Doesn't this just demux kate subtitles as text
without stripping whatever formatting tags kate might support? (I was
under the impression that kate does more than just plain text.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 02/12] bfi: validate sample_rate

2016-10-24 Thread wm4
On Sun, 23 Oct 2016 18:27:02 +0200
Andreas Cadhalpun  wrote:

> A negative sample rate doesn't make sense and triggers assertions in
> av_rescale_rnd.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/bfi.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavformat/bfi.c b/libavformat/bfi.c
> index 568363d..ef4c17d 100644
> --- a/libavformat/bfi.c
> +++ b/libavformat/bfi.c
> @@ -88,6 +88,10 @@ static int bfi_read_header(AVFormatContext * s)
> vstream->codecpar->extradata_size);
>  
>  astream->codecpar->sample_rate = avio_rl32(pb);
> +if (astream->codecpar->sample_rate <= 0) {
> +av_log(s, AV_LOG_ERROR, "Invalid sample rate %d\n", 
> astream->codecpar->sample_rate);
> +return AVERROR_INVALIDDATA;
> +}
>  
>  /* Set up the video codec... */
>  avpriv_set_pts_info(vstream, 32, 1, fps);

Would it make sense to validate codecpars in the generic code (utils.c)?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/4] lavfi: add FFFrameQueue API.

2016-10-24 Thread wm4
On Sun, 23 Oct 2016 12:27:41 +0200
Nicolas George  wrote:

> Signed-off-by: Nicolas George 
> ---
>  libavfilter/Makefile |   1 +
>  libavfilter/framequeue.c | 123 +
>  libavfilter/framequeue.h | 173 
> +++
>  3 files changed, 297 insertions(+)
>  create mode 100644 libavfilter/framequeue.c
>  create mode 100644 libavfilter/framequeue.h
> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 7ed4696..623dd8e 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -18,6 +18,7 @@ OBJS = allfilters.o 
> \
> fifo.o   \
> formats.o\
> framepool.o  \
> +   framequeue.o \
> graphdump.o  \
> graphparser.o\
> opencl_allkernels.o  \
> diff --git a/libavfilter/framequeue.c b/libavfilter/framequeue.c
> new file mode 100644
> index 000..debeab2
> --- /dev/null
> +++ b/libavfilter/framequeue.c
> @@ -0,0 +1,123 @@
> +/*
> + * Generic frame queue
> + * Copyright (c) 2016 Nicolas George
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "framequeue.h"
> +
> +static inline FFFrameBucket *bucket(FFFrameQueue *fq, size_t idx)
> +{
> +return >queue[(fq->tail + idx) & (fq->allocated - 1)];
> +}
> +
> +void ff_framequeue_global_init(FFFrameQueueGlobal *fqg)
> +{
> +}
> +
> +static void check_consistency(FFFrameQueue *fq)
> +{
> +#if ASSERT_LEVEL >= 2
> +uint64_t nb_samples = 0;
> +size_t i;
> +
> +av_assert0(fq->queued == fq->total_frames_head - fq->total_frames_tail);
> +for (i = 0; i < fq->queued; i++)
> +nb_samples += bucket(fq, i)->frame->nb_samples;
> +av_assert0(nb_samples == fq->total_samples_head - 
> fq->total_samples_tail);
> +#endif
> +}
> +
> +void ff_framequeue_init(FFFrameQueue *fq, FFFrameQueueGlobal *fqg)
> +{
> +fq->queue = >first_bucket;
> +fq->allocated = 1;
> +}
> +
> +void ff_framequeue_free(FFFrameQueue *fq)
> +{
> +while (fq->queued) {
> +AVFrame *frame = ff_framequeue_take(fq);
> +av_frame_free();
> +}
> +if (fq->queue != >first_bucket)
> +av_freep(>queue);
> +}
> +
> +int ff_framequeue_add(FFFrameQueue *fq, AVFrame *frame)
> +{
> +FFFrameBucket *b;
> +
> +check_consistency(fq);
> +if (fq->queued == fq->allocated) {
> +if (fq->allocated == 1) {
> +size_t na = 8;
> +FFFrameBucket *nq = av_realloc_array(NULL, na, sizeof(*nq));
> +if (!nq)
> +return AVERROR(ENOMEM);
> +nq[0] = fq->queue[0];
> +fq->queue = nq;
> +fq->allocated = na;
> +} else {
> +size_t na = fq->allocated << 1;
> +FFFrameBucket *nq = av_realloc_array(fq->queue, na, sizeof(*nq));
> +if (!nq)
> +return AVERROR(ENOMEM);
> +if (fq->tail + fq->queued > fq->allocated)
> +memmove(nq + fq->allocated, nq,
> +(fq->tail + fq->queued - fq->allocated) * 
> sizeof(*nq));
> +fq->queue = nq;
> +fq->allocated = na;
> +}
> +}
> +b = bucket(fq, fq->queued);
> +b->frame = frame;
> +fq->queued++;
> +fq->total_frames_head++;
> +fq->total_samples_head += frame->nb_samples;
> +check_consistency(fq);
> +return 0;
> +}
> +
> +AVFrame *ff_framequeue_take(FFFrameQueue *fq)
> +{
> +FFFrameBucket *b;
> +
> +check_consistency(fq);
> +av_assert1(fq->queued);
> +b = bucket(fq, 0);
> +fq->queued--;
> +fq->tail++;
> +fq->tail &= fq->allocated - 1;
> +fq->total_frames_tail++;
> +fq->total_samples_tail += b->frame->nb_samples;
> +check_consistency(fq);
> +return b->frame;
> +}
> +

Re: [FFmpeg-devel] [PATCH 02/13] avcodec: Clear MMX state in ff_thread_report_progress(INT_MAX)

2016-10-24 Thread wm4
On Sat, 22 Oct 2016 22:06:22 -0400
"Ronald S. Bultje"  wrote:

> Hi,
> 
> On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer  > wrote:  
> 
> > This decreases the number of FPU state violations in fate on x86-64 from
> > 309 to 79
> >
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/pthread_frame.c | 3 +++
> >  libavcodec/utils.c | 2 ++
> >  2 files changed, 5 insertions(+)
> >
> > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> > index 7ef5e9f..b6c6027 100644
> > --- a/libavcodec/pthread_frame.c
> > +++ b/libavcodec/pthread_frame.c
> > @@ -473,6 +473,9 @@ void ff_thread_report_progress(ThreadFrame *f, int n,
> > int field)
> >  PerThreadContext *p;
> >  volatile int *progress = f->progress ? (int*)f->progress->data : NULL;
> >
> > +if (n >= INT_MAX)
> > +emms_c();  
> 
> 
> I don't like how INT_MAX is becoming more and more like a magic number.
> This should probably be a new function that is called upon frame (or field)
> decoding completion (or abort), where part of what it does is to set
> progress to INT_MAX.

Indeed, this looks very strange.

Also the ">" in the ">=" is redundant, why isn't it "=="?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.

2016-10-24 Thread wm4
On Sun, 23 Oct 2016 05:37:25 +0200
Michael Niedermayer  wrote:

> Signed-off-by: Michael Niedermayer 
> ---
>  libavutil/x86/emms.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> index 6fda6e2..42c18e2 100644
> --- a/libavutil/x86/emms.h
> +++ b/libavutil/x86/emms.h
> @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
>   * Empty mmx state.
>   * this must be called between any dsp function and float/double code.
>   * for example sin(); dsp->idct_put(); emms_c(); cos()
> + * Note, *alloc() and *free() also use float code in some libc 
> implementations
> + * thus this also applies to them or any function using them.
>   */
>  static av_always_inline void emms_c(void)
>  {

Overly specific and useless information. It's an implementation detail
of 1 specific libc. It could happen to any libc function for any libc
at any time. This just adds noise because of one specific bug.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

2016-10-24 Thread wm4
On Sun, 23 Oct 2016 13:02:01 -0400
"Ronald S. Bultje"  wrote:

> Hi,
> 
> On Sat, Oct 22, 2016 at 11:36 PM, Michael Niedermayer <
> mich...@niedermayer.cc> wrote:  
> 
> > On Sat, Oct 22, 2016 at 10:10:01PM -0400, Ronald S. Bultje wrote:  
> > > Hi,
> > >
> > > general comment about all other dec patches.
> > >
> > > On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer  
> >  > > > wrote:  
> > >  
> > > > Signed-off-by: Michael Niedermayer 
> > > > ---
> > > >  libavcodec/svq1dec.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c
> > > > index 2b72e08..0fe222e 100644
> > > > --- a/libavcodec/svq1dec.c
> > > > +++ b/libavcodec/svq1dec.c
> > > > @@ -744,6 +744,7 @@ static int svq1_decode_frame(AVCodecContext  
> > *avctx,  
> > > > void *data,
> > > >  }
> > > >  }
> > > >  }
> > > > +emms_c();  
> > >
> > >
> > > This is hideous, you're sprinkling emms_c in various places to make some
> > > stupid test pass. The test is morbidly stupid and there is no general
> > > consensus on patterns to be followed as for where to place emms_c.  
> > Someone  
> > > who doesn't know any better will litter each new decoder with 10-20 calls
> > > to emms_c just because he found that other decoders do it in  
> > undocumented,  
> > > unexplained and unclear locations also.
> > >
> > > If you want this to be a "thing", you need to design and document  
> > carefully  
> > > where emms_c is necessary. Then come up with some system that makes this
> > > work by itself.  
> >
> > Your mail sounds quite aggressive to me, did i say something to anger
> > you ? It was certainly not intended
> >
> > About this patchset
> > FFmpeg is broken ATM (both in theory and practice with some libcs),
> > the way MMX code is used is not correct, emms_c()
> > calls are missing and required. The obvious way to fix that
> > (in practice) is adding emms_c() calls where they are missing.
> > I can document why each call is needed, if thats wanted?  
> 
> 
> Your representation of facts is strange, to say the least. Let's explore
> two related claims:
> 
> - (A) ffmpeg is broken in practice when calling musl malloc/free functions
> after calling MMX SIMD functions on x86-32 (which crashes).
> - (B) ffmpeg is broken in theory because we don't clear FPU state (as
> required) at the end of MMX SIMD functions.

I was under the impression that it is UB to have the FPU in MMX state
at any time while in C, not just while e.g. calling the stdlib. Maybe I
got that wrong (how would MMX intrinsics even work?) - can anyone shed
light on the exact requirements? (Possibly again, sorry.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] add hds demuxer

2016-10-24 Thread wm4
On Sat, 15 Oct 2016 15:52:48 +0200
Nicolas George  wrote:

> Le tridi 23 vendémiaire, an CCXXV, wm4 a écrit :
> > XML is very complex  
> 
> This is the usual, and often only, argument raised in this kind of
> situation. And unfortunately, I already addressed it in this very thread.
> 
> XML is very complex, yes: processing instructions, entity definitions, DTSs
> and schemas, namespaces, etc.
> 
> Fortunately, we do not need that. Manifests only use the most basic XML
> features: elements, attributes, text. Parsing that is easy.
> 
> Regards,
> 

Well, I thought the idea was already rejected by multiple people before.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavc/videotoolboxenc: implement a53cc

2016-10-24 Thread Aman Gupta
On Monday, September 19, 2016, Richard Kern  wrote:

>
> On Sep 10, 2016, at 10:33 PM, Aman Gupta  > wrote:
>
>
>
> On Sunday, September 11, 2016, Richard Kern  > wrote:
>
>>
>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta  wrote:
>> >
>> > From: Aman Gupta 
>> >
>> > ---
>> > libavcodec/videotoolboxenc.c | 76 ++
>> --
>> > 1 file changed, 67 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>> > index 4345ca3..859dde9 100644
>> > --- a/libavcodec/videotoolboxenc.c
>> > +++ b/libavcodec/videotoolboxenc.c
>> > @@ -32,6 +32,7 @@
>> > #include "libavutil/pixdesc.h"
>> > #include "internal.h"
>> > #include 
>> > +#include "h264.h"
>> >
>> > #if !CONFIG_VT_BT2020
>> > # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
>> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
>> >
>> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
>> >
>> > +typedef struct ExtraSEI {
>> > +  void *data;
>> > +  size_t size;
>> > +} ExtraSEI;
>> > +
>> > typedef struct BufNode {
>> > CMSampleBufferRef cm_buffer;
>> > +ExtraSEI *sei;
>> > struct BufNode* next;
>> > int error;
>> > } BufNode;
>> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>> > bool flushing;
>> > bool has_b_frames;
>> > bool warned_color_range;
>> > +bool a53_cc;
>> > } VTEncContext;
>> >
>> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
>> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx,
>> int err)
>> > pthread_mutex_unlock(>lock);
>> > }
>> >
>> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>> CMSampleBufferRef *buf)
>> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>> CMSampleBufferRef *buf, ExtraSEI **sei)
>> > {
>> > BufNode *info;
>> >
>> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>> wait, CMSampleBufferRef *buf)
>> > pthread_mutex_unlock(>lock);
>> >
>> > *buf = info->cm_buffer;
>> > +if (sei && *buf) {
>> > +*sei = info->sei;
>> > +} else if (info->sei) {
>> > +if (info->sei->data) av_free(info->sei->data);
>> > +av_free(info->sei);
>> > +}
>> > av_free(info);
>> >
>> > vtctx->frame_ct_out++;
>> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>> wait, CMSampleBufferRef *buf)
>> > return 0;
>> > }
>> >
>> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
>> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef
>> buffer, ExtraSEI *sei)
>> > {
>> > BufNode *info = av_malloc(sizeof(BufNode));
>> > if (!info) {
>> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx,
>> CMSampleBufferRef buffer)
>> >
>> > CFRetain(buffer);
>> > info->cm_buffer = buffer;
>> > +info->sei = sei;
>> > info->next = NULL;
>> >
>> > pthread_mutex_lock(>lock);
>> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
>> > {
>> > AVCodecContext *avctx = ctx;
>> > VTEncContext   *vtctx = avctx->priv_data;
>> > +ExtraSEI *sei = sourceFrameCtx;
>> >
>> > if (vtctx->async_error) {
>> > if(sample_buffer) CFRelease(sample_buffer);
>> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>> > }
>> > }
>> >
>> > -vtenc_q_push(vtctx, sample_buffer);
>> > +vtenc_q_push(vtctx, sample_buffer, sei);
>> > }
>> >
>> > static int get_length_code_size(
>> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
>> > static int vtenc_cm_to_avpacket(
>> > AVCodecContext*avctx,
>> > CMSampleBufferRef sample_buffer,
>> > -AVPacket  *pkt)
>> > +AVPacket  *pkt,
>> > +ExtraSEI  *sei)
>> > {
>> > VTEncContext *vtctx = avctx->priv_data;
>> >
>> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>> > size_t  header_size = 0;
>> > size_t  in_buf_size;
>> > size_t  out_buf_size;
>> > +size_t  sei_nalu_size = 0;
>> > int64_t dts_delta;
>> > int64_t time_base_num;
>> > int nalu_count;
>> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>> > if(status)
>> > return status;
>> >
>> > +if (sei) {
>> > +sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
>> > +}
>> > +
>> > in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>> > out_buf_size = header_size +
>> >in_buf_size +
>> > +   sei_nalu_size +
>> >nalu_count * ((int)sizeof(start_code) -
>> (int)length_code_size);
>> >
>> > status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
>> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>> > length_code_size,
>> > sample_buffer,
>> >