Re: [FFmpeg-devel] [PATCH] interplayacm: increase bitstream buffer size by AV_INPUT_BUFFER_PADDING_SIZE

2016-10-31 Thread Paul B Mahol
On 10/30/16, Andreas Cadhalpun  wrote:
> On 30.10.2016 22:18, Paul B Mahol wrote:
>> On 10/30/16, Andreas Cadhalpun  wrote:
>>> This fixes out-of-bounds reads by the bitstream reader.
>>>
>>> Signed-off-by: Andreas Cadhalpun 
>>> ---
>>>  libavcodec/interplayacm.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/interplayacm.c b/libavcodec/interplayacm.c
>>> index 0486e00..f4a3446 100644
>>> --- a/libavcodec/interplayacm.c
>>> +++ b/libavcodec/interplayacm.c
>>> @@ -72,7 +72,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>>  s->block   = av_calloc(s->block_len, sizeof(int));
>>>  s->wrapbuf = av_calloc(s->wrapbuf_len, sizeof(int));
>>>  s->ampbuf  = av_calloc(0x1, sizeof(int));
>>> -s->bitstream = av_calloc(s->max_framesize, sizeof(*s->bitstream));
>>> +s->bitstream = av_calloc(s->max_framesize +
>>> AV_INPUT_BUFFER_PADDING_SIZE / sizeof(*s->bitstream) + 1,
>>
>> How did you came up with this fix?
>> Little background would help.
>
> The out-of-bounds read happens in get_bits called from linear.
> The buffer passed to init_get_bits8 is &s->bitstream[s->bitstream_index].
> The get_bits documentation says:
> /**
>  * Initialize GetBitContext.
>  * @param buffer bitstream buffer, must be AV_INPUT_BUFFER_PADDING_SIZE
> bytes
>  *larger than the actual read bits because some optimized bitstream
>  *readers read 32 or 64 bit at once and could read over the end
>  * @param byte_size the size of the buffer in bytes
>  * @return 0 on success, AVERROR_INVALIDDATA if the buffer_size would
> overflow.
>  */
> static inline int init_get_bits8(GetBitContext *s, const uint8_t *buffer,
>  int byte_size)
>
> Increasing the buffer size fixed the problem, so the case seems quite clear.
>
> Best regards,
> Andreas
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Stefano Sabatini
On date Thursday 2016-10-27 17:08:50 +0200, Stefano Sabatini encoded:
> On date Wednesday 2016-10-26 01:46:35 +0200, Michael Niedermayer encoded:
> > On Tue, Oct 25, 2016 at 05:07:34PM +0200, Stefano Sabatini wrote:
> [...]
> > > 267580c51d49daf94e73a33175c63ecfed6a0bed  
> > > 0002-lavf-add-ffprobe-demuxer.patch
> > > From 4e0aac4bc00104483859f9950af2ffb15fea6c12 Mon Sep 17 00:00:00 2001
> > > From: Nicolas George 
> > > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > > Subject: [PATCH] lavf: add ffprobe demuxer
> > > 
> > > With several modifications and documentation by Stefano Sabatini
> > > .
> > > 
> > > Signed-off-by: Nicolas George 
> > > ---
> > >  configure|   3 +
> > >  doc/demuxers.texi|  18 +++
> > >  doc/ffprobe-format.texi  | 121 ++
> > >  doc/formats.texi |   1 +
> > >  libavformat/Makefile |   1 +
> > >  libavformat/allformats.c |   1 +
> > >  libavformat/ffprobedec.c | 405 
> > > +++
> > >  7 files changed, 550 insertions(+)
> > >  create mode 100644 doc/ffprobe-format.texi
> > >  create mode 100644 libavformat/ffprobedec.c
> > 
> > did you test this with some fuzzer ?
> > 
> > no further comments from me
> > Acked-by: Michael
> 

> Good idea, found a few crashes with afl. I'll let it run again and
> fix found issues. Then I'll push in a few days if there are no more
> comments.

Fixed a few hangs and crashes found with afl. Will try also to add a
fate test.
>From 7f209e27aa33e8f43444e5cfc44c68f664b69e06 Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Sat, 11 Jan 2014 19:42:41 +0100
Subject: [PATCH] lavf: add ffprobe demuxer

With several modifications and documentation by Stefano Sabatini
.

Signed-off-by: Nicolas George 
---
 configure|   3 +
 doc/demuxers.texi|  18 ++
 doc/ffprobe-format.texi  | 121 +
 doc/formats.texi |   1 +
 libavformat/Makefile |   1 +
 libavformat/allformats.c |   1 +
 libavformat/ffprobedec.c | 439 +++
 7 files changed, 584 insertions(+)
 create mode 100644 doc/ffprobe-format.texi
 create mode 100644 libavformat/ffprobedec.c

diff --git a/configure b/configure
index 72ffaea..71b9d73 100755
--- a/configure
+++ b/configure
@@ -3349,6 +3349,9 @@ for n in $COMPONENT_LIST; do
 eval ${n}_if_any="\$$v"
 done
 
+# Disable ffprobe demuxer for security concerns
+disable ffprobe_demuxer
+
 enable $ARCH_EXT_LIST
 
 die_unknown(){
diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 2934a1c..0e6dfbd 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -243,6 +243,24 @@ file subdir/file-2.wav
 @end example
 @end itemize
 
+@section ffprobe
+
+ffprobe internal demuxer. It is able to demux files in the format
+specified in the @ref{FFprobe demuxer format} chapter.
+
+For security reasons this demuxer is disabled by default, should be
+enabled though the @code{--enable-demuxer=ffprobe} configure option.
+
+This format is useful to generate streams in a textual format, easily
+generated with scripting or by hand (by editing the output of
+@command{ffprobe}).
+
+In particular, it can be also used to inject data stream generated by
+scripts in the @command{ffmpeg} output, for example with the command:
+@example
+ffmpeg -i INPUT -i data.ffprobe -map 0:0 -map 0:1 -map 1:0 -codec:d copy OUTPUT
+@end example
+
 @section flv
 
 Adobe Flash Video Format demuxer.
diff --git a/doc/ffprobe-format.texi b/doc/ffprobe-format.texi
new file mode 100644
index 000..95ac644
--- /dev/null
+++ b/doc/ffprobe-format.texi
@@ -0,0 +1,121 @@
+@anchor{FFprobe demuxer format}
+@chapter FFprobe demuxer format
+@c man begin FFPROBE DEMUXER FORMAT
+
+The ffprobe demuxer format is inspired by the output generated by the
+ffprobe default format.
+
+It consists of several sections (@samp{FORMAT}, @samp{STREAM},
+@samp{PACKET}). Each section starts with a single line in the format
+@samp{[SECTION]} and ends with the corresponding line
+@samp{[/SECTION]}, where @samp{SECTION} is the name of the section.
+
+A well-formed file consists of an initial @samp{FORMAT} section,
+followed by several @samp{STREAM} sections (one per stream), and
+several @samp{PACKET} sections.
+
+Each section contains a sequence of lines in the form
+@samp{key=value}. In the case of data the key and the value must
+stay on different lines.
+
+Unrecognized values are discarded.
+
+This format can be read by the @code{ffprobe} demuxer. It is an
+internal format and thus should not be used for archiving purposes.
+
+The following sections document the fields accepted by each section.
+
+@section FORMAT
+
+@table @samp
+@item nb_streams
+the number of supported streams
+@end table
+
+@section STREAM
+@table @samp
+@item index
+the index number
+@item codec_name
+the codec name (the name must be an accepted FFmpeg codec name
+@item time_base
+Specify the time_base as @samp{NUM/DEN}, this is the time base used to
+specify the timestamps in the correspo

Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

2016-10-31 Thread Kevin Wheatley
On Sun, Oct 30, 2016 at 1:18 PM, Ronald S. Bultje  wrote:
> Hmm... So, the wikipedia page https://en.wikipedia.org/wiki/DCI-P3 refers
> to the two whitepoints here as DCI-P3 D65 and DCI-P3 Theater. Calling one
> D65 and the other DCI seems confusing in that light (assuming the wikipedia
> page is correct). I'd call it THEATER or DCI_P3_THEATER, to distinguish it
> from DCI-P3 D65. Is that OK?

In the industry people just call it the DCI P3 white point (or 'Urgh')
it is not limited to theater usage, you might consider it the
calibration white point for the reference projector, so
WP_DCI_P3_REFERENCE might be better, but that is a little long.

I'd go for something like WP_DCI_P3 it is not really ambiguous.

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


Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

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

On Mon, Oct 31, 2016 at 5:50 AM, Kevin Wheatley 
wrote:

> On Sun, Oct 30, 2016 at 1:18 PM, Ronald S. Bultje 
> wrote:
> > Hmm... So, the wikipedia page https://en.wikipedia.org/wiki/DCI-P3
> refers
> > to the two whitepoints here as DCI-P3 D65 and DCI-P3 Theater. Calling one
> > D65 and the other DCI seems confusing in that light (assuming the
> wikipedia
> > page is correct). I'd call it THEATER or DCI_P3_THEATER, to distinguish
> it
> > from DCI-P3 D65. Is that OK?
>
> In the industry people just call it the DCI P3 white point (or 'Urgh')
> it is not limited to theater usage, you might consider it the
> calibration white point for the reference projector, so
> WP_DCI_P3_REFERENCE might be better, but that is a little long.
>
> I'd go for something like WP_DCI_P3 it is not really ambiguous.


Hm... OK with me (though not ideal, but what do I know). Vittorio, OK also?
I can modify patch so you don't have to resend.

Ronald
___
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-31 Thread Derek Buitenhuis
On 10/27/2016 10:48 PM, Sasi Inguva wrote:
> gentle ping.
> 
> Thanks!

A ping from me, too.

- Derek
___
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-31 Thread Rostislav Pehlivanov
On 31 October 2016 at 13:13, Derek Buitenhuis 
wrote:

> On 10/27/2016 10:48 PM, Sasi Inguva wrote:
> > gentle ping.
> >
> > Thanks!
>
> A ping from me, too.
>
> - Derek
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

The tests pass on my machine, pushed after Derek said it looks correct.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread wm4
On Mon, 31 Oct 2016 09:51:10 +0100
Stefano Sabatini  wrote:

> On date Thursday 2016-10-27 17:08:50 +0200, Stefano Sabatini encoded:
> > On date Wednesday 2016-10-26 01:46:35 +0200, Michael Niedermayer encoded:
> > > On Tue, Oct 25, 2016 at 05:07:34PM +0200, Stefano Sabatini wrote:
> > [...]
> > > > 267580c51d49daf94e73a33175c63ecfed6a0bed  
> > > > 0002-lavf-add-ffprobe-demuxer.patch
> > > > From 4e0aac4bc00104483859f9950af2ffb15fea6c12 Mon Sep 17 00:00:00 2001
> > > > From: Nicolas George 
> > > > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > > > Subject: [PATCH] lavf: add ffprobe demuxer
> > > > 
> > > > With several modifications and documentation by Stefano Sabatini
> > > > .
> > > > 
> > > > Signed-off-by: Nicolas George 
> > > > ---
> > > >  configure|   3 +
> > > >  doc/demuxers.texi|  18 +++
> > > >  doc/ffprobe-format.texi  | 121 ++
> > > >  doc/formats.texi |   1 +
> > > >  libavformat/Makefile |   1 +
> > > >  libavformat/allformats.c |   1 +
> > > >  libavformat/ffprobedec.c | 405 
> > > > +++
> > > >  7 files changed, 550 insertions(+)
> > > >  create mode 100644 doc/ffprobe-format.texi
> > > >  create mode 100644 libavformat/ffprobedec.c
> > > 
> > > did you test this with some fuzzer ?
> > > 
> > > no further comments from me
> > > Acked-by: Michael
> > 
> 
> > Good idea, found a few crashes with afl. I'll let it run again and
> > fix found issues. Then I'll push in a few days if there are no more
> > comments.
> 
> Fixed a few hangs and crashes found with afl. Will try also to add a
> fate test.

You still didn't add my Nacked-by header.

I very strongly disagree with this patch, but consider myself
over-voted.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] Add experimental muxing support for FLAC in ISO BMFF (MP4).

2016-10-31 Thread James Almer
On 10/31/2016 3:27 AM, Matthew Gregan wrote:
> Based on the draft spec at 
> https://git.xiph.org/?p=flac.git;a=blob;f=doc/isoflac.txt
> 
> '-strict -2' is required to create files in this format.
> 
> Signed-off-by: Matthew Gregan 
> ---
>  libavformat/isom.c   |  2 ++
>  libavformat/movenc.c | 43 +--
>  2 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/isom.c b/libavformat/isom.c
> index ab79e22..aacbe43 100644
> --- a/libavformat/isom.c
> +++ b/libavformat/isom.c
> @@ -60,6 +60,7 @@ const AVCodecTag ff_mp4_obj_type[] = {
>  { AV_CODEC_ID_EAC3, 0xA6 },
>  { AV_CODEC_ID_DTS , 0xA9 }, /* mp4ra.org */
>  { AV_CODEC_ID_VP9 , 0xC0 }, /* nonstandard, update when there is 
> a standard value */
> +{ AV_CODEC_ID_FLAC, 0xC1 }, /* nonstandard, update when there is 
> a standard value */
>  { AV_CODEC_ID_TSCC2   , 0xD0 }, /* nonstandard, camtasia uses it */
>  { AV_CODEC_ID_VORBIS  , 0xDD }, /* nonstandard, gpac uses it */
>  { AV_CODEC_ID_DVD_SUBTITLE, 0xE0 }, /* nonstandard, see 
> unsupported-embedded-subs-2.mp4 */
> @@ -345,6 +346,7 @@ const AVCodecTag ff_codec_movaudio_tags[] = {
>  { AV_CODEC_ID_WMAV2,   MKTAG('W', 'M', 'A', '2') },
>  { AV_CODEC_ID_EVRC,MKTAG('s', 'e', 'v', 'c') }, /* 3GPP2 */
>  { AV_CODEC_ID_SMV, MKTAG('s', 's', 'm', 'v') }, /* 3GPP2 */
> +{ AV_CODEC_ID_FLAC,MKTAG('f', 'L', 'a', 'C') }, /* 
> nonstandard */
>  { AV_CODEC_ID_NONE, 0 },
>  };
>  
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 6228192..d77250e 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -654,6 +654,27 @@ static int mov_write_wfex_tag(AVFormatContext *s, 
> AVIOContext *pb, MOVTrack *tra
>  return update_size(pb, pos);
>  }
>  
> +static int mov_write_dfla_tag(AVIOContext *pb, MOVTrack *track)
> +{
> +const size_t FLAC_STREAMINFO_SIZE = 34;
> +int64_t pos = avio_tell(pb);
> +avio_wb32(pb, 0);
> +ffio_wfourcc(pb, "dfLa");
> +avio_w8(pb, 0); /* version */
> +avio_wb24(pb, 0); /* flags */
> +
> +/* Expect the encoder to pass a METADATA_BLOCK_TYPE_STREAMINFO. */

The encoder may (or most likely will) also pass an updated streaminfo as packet
side data.
See the flac muxer, you'll have to do the same here.

> +if (track->par->extradata_size != FLAC_STREAMINFO_SIZE)
> +  return AVERROR_INVALIDDATA;
> +
> +/* TODO: Write other METADATA_BLOCK_TYPEs if the encoder makes them 
> available. */
> +avio_w8(pb, 1 << 7 | 0); /* LastMetadataBlockFlag << 7 | BlockType 
> (STREAMINFO) */
> +avio_wb24(pb, track->par->extradata_size); /* Length */
> +avio_write(pb, track->par->extradata, track->par->extradata_size); /* 
> BlockData[Length] */
> +
> +return update_size(pb, pos);
> +}
> +
>  static int mov_write_chan_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack 
> *track)
>  {
>  uint32_t layout_tag, bitmap;
> @@ -963,8 +984,13 @@ static int mov_write_audio_tag(AVFormatContext *s, 
> AVIOContext *pb, MOVMuxContex
>  avio_wb16(pb, 16);
>  avio_wb16(pb, track->audio_vbr ? -2 : 0); /* compression ID */
>  } else { /* reserved for mp4/3gp */
> -avio_wb16(pb, 2);
> -avio_wb16(pb, 16);
> +if (track->par->codec_id == AV_CODEC_ID_FLAC) {
> +avio_wb16(pb, track->par->channels);
> +avio_wb16(pb, av_get_bytes_per_sample(track->par->format) * 
> 8);
> +} else {
> +avio_wb16(pb, 2);
> +avio_wb16(pb, 16);
> +}
>  avio_wb16(pb, 0);
>  }
>  
> @@ -1009,6 +1035,8 @@ static int mov_write_audio_tag(AVFormatContext *s, 
> AVIOContext *pb, MOVMuxContex
>  mov_write_extradata_tag(pb, track);
>  else if (track->par->codec_id == AV_CODEC_ID_WMAPRO)
>  mov_write_wfex_tag(s, pb, track);
> +else if (track->par->codec_id == AV_CODEC_ID_FLAC)
> +mov_write_dfla_tag(pb, track);
>  else if (track->vos_len > 0)
>  mov_write_glbl_tag(pb, track);
>  
> @@ -1177,6 +1205,7 @@ static int mp4_get_codec_tag(AVFormatContext *s, 
> MOVTrack *track)
>  else if (track->par->codec_id == AV_CODEC_ID_DIRAC) tag = 
> MKTAG('d','r','a','c');
>  else if (track->par->codec_id == AV_CODEC_ID_MOV_TEXT)  tag = 
> MKTAG('t','x','3','g');
>  else if (track->par->codec_id == AV_CODEC_ID_VC1)   tag = 
> MKTAG('v','c','-','1');
> +else if (track->par->codec_id == AV_CODEC_ID_FLAC)  tag = 
> MKTAG('f','L','a','C');
>  else if (track->par->codec_type == AVMEDIA_TYPE_VIDEO)  tag = 
> MKTAG('m','p','4','v');
>  else if (track->par->codec_type == AVMEDIA_TYPE_AUDIO)  tag = 
> MKTAG('m','p','4','a');
>  else if (track->par->codec_id == AV_CODEC_ID_DVD_SUBTITLE)  tag = 
> MKTAG('m','p','4','s');
> @@ -5733,6 +5762,16 @@ static int mov_init(AVFo

[FFmpeg-devel] why the option 'timeout' in native rtmp plugin doesn't work

2016-10-31 Thread qw
Hi,


1)
I want to open and read some rtmp live stream, and don't like wait indefinite 
time for some rtmp stream when there is no rtmp stream to come. Therefore, the 
option of 'timeout' is used to set the longest waiting time. But it didn't 
work, and avformat_open_input() reported as follows:


[rtmp @ 0x1430580] Cannot open connection 
tcp://localhost:1935?listen&listen_timeout=5000


The source code is shown as follows:


AVDictionary *pFormatOpts = NULL;
AVFormatContext *pIfmtCtx;
char pInRtmpUrl[MAX_RTMP_URL_LENGTH];


av_dict_set(&pFormatOpts, "rtmp_live", "-1", 0);
av_dict_set(&pFormatOpts, "rtmp_listen", "1", 0);
av_dict_set(&pFormatOpts, "timeout", "5", 0);


avformat_open_input(&pIfmtCtx, pInRtmpUrl, NULL, &pFormatOpts)


Why the rtmp setting doesn't work?


2)
If I want to set the timeout for av_read_frame()/av_interleaved_write_frame() 
to prevent the function from waiting indefinite time for reading/writing 
avpackets, is there some options for the native rtmp plugin?




Thanks!


Regards


Andrew






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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Jean-Baptiste Kempf
Hi,

On Mon, 31 Oct 2016, at 07:03, wm4 wrote:
> I very strongly disagree with this patch, but consider myself
> over-voted.


I strongly disagree with this patch too.

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 4/4] qsv: Merge libav implementation

2016-10-31 Thread Hendrik Leppkes
On Wed, Oct 26, 2016 at 9:50 PM, Mark Thompson  wrote:
> Merged as-at libav 398f015, and therefore includes outstanding
> skipped merges 04b17ff and 130e1f1.
>
> All features not in libav are preserved, and no options change.
> ---

LGTM, this should make further work on this much easier and bring it
back to a point where its actually stable - and integrates with the
existing HW landscape (hwcontext et al).

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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit :
> I strongly disagree with this patch too.

We do not care about the number of people who disagree, we only care
about practical arguments.

IIRC, the last argument on the matter was mine:
http://ffmpeg.org/pipermail/ffmpeg-devel/2016-September/200317.html

If you, or anyone else opposing it, has counter-arguments, please give
them. Otherwise, the patch should go in once it is clean.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 11:20 AM, Nicolas George wrote:
> Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit :
>> I strongly disagree with this patch too.
> 
> We do not care about the number of people who disagree, we only care
> about practical arguments.

That's a curious thing to say when we have a voting committee to
deal with polarizing subjects that went beyond practical arguments.

> 
> IIRC, the last argument on the matter was mine:
> http://ffmpeg.org/pipermail/ffmpeg-devel/2016-September/200317.html
> 
> If you, or anyone else opposing it, has counter-arguments, please give
> them. Otherwise, the patch should go in once it is clean.
> 
> Regards,
> 
> 
> 
> ___
> 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: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> That's a curious thing to say when we have a voting committee to
> deal with polarizing subjects that went beyond practical arguments.

The voting committee is there for when arguments are failed: when there
are significant pros and cons on each side and deciding which one is
better and which one is worse is a matter of policy and taste and not a
technical matter.

People who try to vote instead of giving arguments are trying to abuse
the power of the voting committee.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Jean-Baptiste Kempf
Hi,

On Mon, 31 Oct 2016, at 07:20, Nicolas George wrote:
> Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit :
> > I strongly disagree with this patch too.
> 
> If you, or anyone else opposing it, has counter-arguments, please give
> them. Otherwise, the patch should go in once it is clean.

ffprobe is not a video/audio format.
It has no public specification, is made up and completely arbitrary
format.
It will not be used by the large majority of applications.

I understand very well the need, I disagree it should go inside the
(de)muxing library.

Best regards

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 11:30 AM, Nicolas George wrote:
> Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
>> That's a curious thing to say when we have a voting committee to
>> deal with polarizing subjects that went beyond practical arguments.
> 
> The voting committee is there for when arguments are failed: when there
> are significant pros and cons on each side and deciding which one is
> better and which one is worse is a matter of policy and taste and not a
> technical matter.
> 
> People who try to vote instead of giving arguments are trying to abuse
> the power of the voting committee.

Afaics, the cons argumented were that this "demuxer" doesn't fit the
criteria of a libavformat module. It's not demultiplexing a multimedia
file.
It, unless i misread it, is just reconstructing one out of a non-binary
dump.

This should be implemented as a separate tool using libavformat, and
not as a libavformat module.

See liboggz's oggz-dump tool.

> 
> Regards,
> 
> 
> 
> ___
> 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: add ffprobe demuxer

2016-10-31 Thread Stefano Sabatini
On date Monday 2016-10-31 11:27:17 -0300, James Almer encoded:
> On 10/31/2016 11:20 AM, Nicolas George wrote:
> > Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit :
> >> I strongly disagree with this patch too.
> > 
> > We do not care about the number of people who disagree, we only care
> > about practical arguments.
> 
> That's a curious thing to say when we have a voting committee to
> deal with polarizing subjects that went beyond practical arguments.

So, in practical terms, do you want me to start a formal vote about
the inclusion of the format?

I remember that in the last version the format is disabled by default
(if people prefer I can add a dependency on an --enable-unsafe
configure switch).

I'm still convinced it could be useful for some testing scenarios and
for data injection, but I'll commit to the decision of the voting
committee in case it is rejected.
-- 
FFmpeg = Frightening & Fancy Marvellous Picky Extroverse Ghost
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, Stefano Sabatini a écrit :
> So, in practical terms, do you want me to start a formal vote about
> the inclusion of the format?

NO! Until ten minutes ago, there was no actual arguments against. The
technical discussion just started.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Jean-Baptiste Kempf:
> ffprobe is not a video/audio format.
> It has no public specification, is made up and completely arbitrary
> format.
> It will not be used by the large majority of applications.

Indeed. The same goes for a lot of obscure formats used to decode game
cutscene data. At least, with ffprobe, we did not have to
reverse-engineer the format.

> I understand very well the need,

Thanks.

>  I disagree it should go inside the
> (de)muxing library.

Why? Is it just a matter of taste for you, or do you see practical
drawbacks?

James Almer:
> Afaics, the cons argumented were that this "demuxer" doesn't fit the
> criteria of a libavformat module.

I do not know the criteria of a libavformat module. Can you enlighten
me?

>   It's not demultiplexing a multimedia
> file.

Uh? I think this is exactly what all demuxers do. They can not do
anything else.

> It, unless i misread it, is just reconstructing one out of a non-binary
> dump.

The "dump" is a file, it contains multimedia data: it is a multimedia
file.

> This should be implemented as a separate tool using libavformat, and
> not as a libavformat module.

It could be done. But, once again: why? Is it just a matter of taste for
you, or do you see practical drawbacks?

Making a tool to build a real file from the text dump allows to get the
feature without intruding lavf, that is true.

But it would require exactly the same amount of code to parse the dump,
plus the code to mux the file: more code, more maintenance.

Plus, in terms of debugging, it adds three extra points of failure: the
muxer for the intermediate format (used in the conversion tool), the
demuxer for the intermediate format (used in ffmpeg.c) and the
limitations of the intermediate format itself.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] avfilter/af_silenceremove: add optional tone when silence is removed

2016-10-31 Thread Greg Rowe

On 2016-10-18 2:20 PM, Michael Niedermayer wrote:

On Tue, Oct 18, 2016 at 12:46:56PM -0400, Greg Rowe wrote:

see
libavfilter/asrc_sine.c
this code should probably be reused / factored
(note, any code moving/factoring of existing code should be in a
seperate patch)


Since silenceremove works only on AV_SAMPLE_FMT_DBL is it OK to use
floating point for the generated tone or is it recommended to create
the tone and then convert it to double samples or some other
approach?


can you write a fate test for the filter which is portable accross
platforms ?

every filter should ideally have a fate test.
If you can write a working an portable fate test with float sine
then i have no objections to it


Do you have ideas on how I could implement this feature while avoiding 
floating point (or at least to implemented the feature in a portable 
manner)?  I agree that it would be better but I'm not sure how to do it.


I could change the entire silence remove filter to avoid floating point 
ops but that would be a big change and I don't think that's a good idea.


Would it work if I created the tone using asrc_since.c and then 
converted that to double samples within the filter?  (rather than my 
existing approach which generates the tone as doubles using sin()).


Thanks,
Greg

--
Greg Rowe
www.shoretel.com


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


Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

2016-10-31 Thread Vittorio Giovara
On Mon, Oct 31, 2016 at 7:43 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Mon, Oct 31, 2016 at 5:50 AM, Kevin Wheatley 
> wrote:
>>
>> On Sun, Oct 30, 2016 at 1:18 PM, Ronald S. Bultje 
>> wrote:
>> > Hmm... So, the wikipedia page https://en.wikipedia.org/wiki/DCI-P3
>> > refers
>> > to the two whitepoints here as DCI-P3 D65 and DCI-P3 Theater. Calling
>> > one
>> > D65 and the other DCI seems confusing in that light (assuming the
>> > wikipedia
>> > page is correct). I'd call it THEATER or DCI_P3_THEATER, to distinguish
>> > it
>> > from DCI-P3 D65. Is that OK?
>>
>> In the industry people just call it the DCI P3 white point (or 'Urgh')
>> it is not limited to theater usage, you might consider it the
>> calibration white point for the reference projector, so
>> WP_DCI_P3_REFERENCE might be better, but that is a little long.
>>
>> I'd go for something like WP_DCI_P3 it is not really ambiguous.
>
>
> Hm... OK with me (though not ideal, but what do I know). Vittorio, OK also?
> I can modify patch so you don't have to resend.

I find it a little long and not less confusing than my initial WP_DCI,
among all the alternatives I liked the THEATER one the most. If that's
a no-go, how about we could settle for WP_PROJ maybe?
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2016-10-31 Thread Nicolas George
Le nonidi 9 brumaire, an CCXXV, Steven Liu a écrit :
>  I saw ffmpeg have no HDS and DASH demuxer, and all of them's format is
> use xml, maybe this parser is a very useful parser, what about the basic
> xml :-D

The Timed Text Markup Language, a subtitle format used by Youtube and
possibly a few others, is based on XML too.

I have started working on a simple XML parser, but Michael quickly found
a bug in my attempt to remove the recursiveness in libavfilter, and I
consider it to be the highest priority. Therefore, I stopped shortly
after implementing the framework API and input buffer handling.

The API I have chosen is a push-pull one similar to the new BSF and
codecs API: av_xml_parser_feed() then loop on
av_xml_parser_get_next_event(). The events would be the start of an
element, a key-value attribute, the end of an element, a closing element
or a text node.

Higher-level utilities are easy to build on top of this API: returning a
single event for an element and all its attributes, reading from an AVIO
context.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH v2] libavformat/tee: Add fifo support for tee

2016-10-31 Thread Nicolas George
Le sextidi 26 vendémiaire, an CCXXV, sebechlebsky...@gmail.com a écrit :
> From: Jan Sebechlebsky 
> 
> Signed-off-by: Jan Sebechlebsky 
> ---
>  Thanks for noticing, I've fixed the patch 
>  (also some minor formatting issues I've noticed).
> 
>  doc/muxers.texi   | 20 +
>  libavformat/tee.c | 87 
> ++-
>  2 files changed, 106 insertions(+), 1 deletion(-)

Sorry for the delay, I forgot I had this to look at.

> 
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index dbe53f5..7b4e165 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -1473,6 +1473,7 @@ Specify whether to remove all fragments when finished. 
> Default 0 (do not remove)
>  
>  @end table
>  
> +@anchor{fifo}
>  @section fifo
>  
>  The fifo pseudo-muxer allows the separation of encoding and muxing by using
> @@ -1580,6 +1581,18 @@ with the tee muxer; encoding can be a very expensive 
> process. It is not
>  useful when using the libavformat API directly because it is then possible
>  to feed the same packets to several muxers directly.
>  
> +@table @option
> +
> +@item use_fifo @var{bool}
> +If set to 1, slave outputs will be processed in separate thread using 
> @ref{fifo}
> +muxer. This allows to compensate for different speed/latency/reliability of
> +outputs and setup transparent recovery. By default this feature is turned 
> off.
> +
> +@item fifo_options
> +Options to pass to fifo pseudo-muxer instances. See @ref{fifo}.
> +
> +@end table
> +
>  The slave outputs are specified in the file name given to the muxer,
>  separated by '|'. If any of the slave name contains the '|' separator,
>  leading or trailing spaces or any special character, it must be
> @@ -1601,6 +1614,13 @@ output name suffix.
>  Specify a list of bitstream filters to apply to the specified
>  output.
>  
> +@item use_fifo @var{bool}
> +This allows to override tee muxer use_fifo option for individual slave muxer.
> +
> +@item fifo_options
> +This allows to override tee muxer fifo_options for individual slave muxer.
> +See @ref{fifo}.
> +
>  It is possible to specify to which streams a given bitstream filter
>  applies, by appending a stream specifier to the option separated by
>  @code{/}. @var{spec} must be a stream specifier (see @ref{Format
> diff --git a/libavformat/tee.c b/libavformat/tee.c
> index d59ad4d..c668e95 100644
> --- a/libavformat/tee.c
> +++ b/libavformat/tee.c
> @@ -40,6 +40,8 @@ typedef struct {
>  AVBSFContext **bsfs; ///< bitstream filters per stream
>  
>  SlaveFailurePolicy on_fail;
> +int use_fifo;
> +AVDictionary *fifo_options;
>  
>  /** map from input to output streams indexes,
>   * disabled output streams are set to -1 */
> @@ -52,15 +54,28 @@ typedef struct TeeContext {
>  unsigned nb_slaves;
>  unsigned nb_alive;
>  TeeSlave *slaves;
> +int use_fifo;
> +AVDictionary *fifo_options;
> +char *fifo_options_str;
>  } TeeContext;
>  
>  static const char *const slave_delim = "|";
>  static const char *const slave_bsfs_spec_sep = "/";
>  static const char *const slave_select_sep = ",";
>  
> +#define OFFSET(x) offsetof(TeeContext, x)
> +static const AVOption options[] = {
> +{"use_fifo", "Use fifo pseudo-muxer to separate actual muxers from 
> encoder",
> + OFFSET(use_fifo), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 
> AV_OPT_FLAG_ENCODING_PARAM},
> +{"fifo_options", "fifo pseudo-muxer options", 
> OFFSET(fifo_options_str),
> + AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, 
> AV_OPT_FLAG_ENCODING_PARAM},
> +{NULL}
> +};
> +
>  static const AVClass tee_muxer_class = {
>  .class_name = "Tee muxer",
>  .item_name  = av_default_item_name,
> +.option = options,
>  .version= LIBAVUTIL_VERSION_INT,
>  };
>  
> @@ -81,6 +96,27 @@ static inline int parse_slave_failure_policy_option(const 
> char *opt, TeeSlave *t
>  return AVERROR(EINVAL);
>  }
>  
> +static int parse_slave_fifo_options(const char *use_fifo,
> +const char *fifo_options, TeeSlave 
> *tee_slave)
> +{
> +int ret = 0;
> +
> +if (use_fifo) {

> +if (av_match_name(use_fifo, "true,y,yes,enable,enabled,on,1")) {
> +tee_slave->use_fifo = 1;
> +} else if (av_match_name(use_fifo, 
> "false,n,no,disable,disabled,off,0")) {

I am not happy about the duplication of the tests from
set_string_bool(), but it cannot be avoided for now without more
unrelated work.

(I really with each option type came with the corresponding silent and
verbose parsing and dump functions. Unfortunately, this discipline was
not kept in the past.)

> +tee_slave->use_fifo = 0;
> +} else {
> +return AVERROR(EINVAL);
> +}
> +}
> +
> +if (fifo_options)
> +ret = av_dict_parse_string(&tee_slave->fifo_options, fifo_options, 
> "=", ":", 0);
> +
> +return ret;
> +}
> +
>  static int close_slave(TeeSlave *tee_slave)
>  {
> 

Re: [FFmpeg-devel] [PATCH] news: add final report for summer of code 2016

2016-10-31 Thread Thomas Volkert

On 31.10.2016 04:47, Reynaldo H. Verdejo Pinochet wrote:
> Thanks for the comments and corrections. Fixed the typos and pushed as:
>
> commit f06598a8e1fcccef8c38a657162db309773d1515
> Author: Reynaldo H. Verdejo Pinochet 
> Date:   Sun Oct 30 01:37:26 2016 -0700
>
> news: add final report for summer of code 2016
>

..also published on Facebook.

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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread wm4
On Mon, 31 Oct 2016 15:20:15 +0100
Nicolas George  wrote:

> Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit :
> > I strongly disagree with this patch too.  
> 
> We do not care about the number of people who disagree, we only care
> about practical arguments.
> 
> IIRC, the last argument on the matter was mine:
> http://ffmpeg.org/pipermail/ffmpeg-devel/2016-September/200317.html
> 
> If you, or anyone else opposing it, has counter-arguments, please give
> them. Otherwise, the patch should go in once it is clean.
> 

I don't have the energy for such long-winded technological discussions
which (predictably) lead to nothing. I just want my disagreement noted.
If we have Reviewed-by etc. headers, why not Nacked-by ones?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, wm4 a écrit :
> I don't have the energy for such long-winded technological discussions
> which (predictably) lead to nothing. I just want my disagreement noted.

Well, you noted it, and I personally do not care at all without
arguments.

> If we have Reviewed-by etc. headers, why not Nacked-by ones?

Because each commit would require approximately seven billion Nacked-by
entries.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 11:57 AM, Nicolas George wrote:
> Jean-Baptiste Kempf:
>> ffprobe is not a video/audio format.
>> It has no public specification, is made up and completely arbitrary
>> format.
>> It will not be used by the large majority of applications.
> 
> Indeed. The same goes for a lot of obscure formats used to decode game
> cutscene data. At least, with ffprobe, we did not have to
> reverse-engineer the format.
> 
>> I understand very well the need,
> 
> Thanks.
> 
>> I disagree it should go inside the
>> (de)muxing library.
> 
> Why? Is it just a matter of taste for you, or do you see practical
> drawbacks?
> 
> James Almer:
>> Afaics, the cons argumented were that this "demuxer" doesn't fit the
>> criteria of a libavformat module.
> 
> I do not know the criteria of a libavformat module. Can you enlighten
> me?
> 
>>  It's not demultiplexing a multimedia
>> file.
> 
> Uh? I think this is exactly what all demuxers do. They can not do
> anything else.
> 
>> It, unless i misread it, is just reconstructing one out of a non-binary
>> dump.
> 
> The "dump" is a file, it contains multimedia data: it is a multimedia
> file.

Someone will come up with an hexdump demuxer next and call hexdump output
a multimedia file, if we follow your views. Or an XML parser for a similarly
arbitrary custom format they happened to devise. Or maybe oggz-dump's output.

While for the former we can tell them to go away, we wouldn't have many
arguments against the latter two if this patch goes in. Because telling them
we don't want their custom format after this one was committed would be
hypocritical, biased, and a decision as arbitrary as the actual hypothetical
format.

> 
>> This should be implemented as a separate tool using libavformat, and
>> not as a libavformat module.
> 
> It could be done. But, once again: why? Is it just a matter of taste for
> you, or do you see practical drawbacks?

Aside from the "arbitrary markup format" argument, the fact it's disabled by
default for security reasons doesn't exactly inspire confidence.
It would afaik be a first for an internal module that doesn't depend on
external factors.

Besides, nothing even /muxes/ this format to begin with. The doxy states you
should take the ffprobe standard output and edit it by hand or with a script
until it looks like something this demuxer can digest.

> 
> Making a tool to build a real file from the text dump allows to get the
> feature without intruding lavf, that is true.
> 
> But it would require exactly the same amount of code to parse the dump,
> plus the code to mux the file: more code, more maintenance.

Which so happens to be what every libav* user has to do for their projects.
Write a program using the libraries' public API to read and write files.

We have plenty of times refused code people wanted to dump into libavformat
and libavcodec because it was the easiest way for them to get what they
needed done. There was this absolutely insane youtube-dl "demuxer" that still
gives me the chills.
We have also plenty of times let things go in, even with several developers
against it. Lets try to not do it again.

> 
> Plus, in terms of debugging, it adds three extra points of failure: the
> muxer for the intermediate format (used in the conversion tool), the
> demuxer for the intermediate format (used in ffmpeg.c) and the
> limitations of the intermediate format itself.
> 
> Regards,
> 
> 
> 
> ___
> 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: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 1:27 PM, Nicolas George wrote:
> Le decadi 10 brumaire, an CCXXV, wm4 a écrit :
>> I don't have the energy for such long-winded technological discussions
>> which (predictably) lead to nothing. I just want my disagreement noted.
> 
> Well, you noted it, and I personally do not care at all without
> arguments.
> 
>> If we have Reviewed-by etc. headers, why not Nacked-by ones?
> 
> Because each commit would require approximately seven billion Nacked-by
> entries.

So you're saying no matter how many people veto a patch it means shit and
they will be committed anyway because the writer of the patch and one other
person want to?

One author, one positive review and seven billion Nack means the commit is
approved?

> 
> Regards,
> 
> 
> 
> ___
> 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: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> So you're saying no matter how many people veto a patch it means shit and
> they will be committed anyway because the writer of the patch and one other
> person want to?
> 
> One author, one positive review and seven billion Nack means the commit is
> approved?

Without arguments, yes, nacks mean nothing.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 1:36 PM, Nicolas George wrote:
> Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
>> So you're saying no matter how many people veto a patch it means shit and
>> they will be committed anyway because the writer of the patch and one other
>> person want to?
>>
>> One author, one positive review and seven billion Nack means the commit is
>> approved?
> 
> Without arguments, yes, nacks mean nothing.

You not liking an argument doesn't mean the argument doesn't exist.

> 
> Regards,
> 
> 
> 
> ___
> 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: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> You not liking an argument doesn't mean the argument doesn't exist.

Indeed.

On the other hand, someone saying they do not intend to give arguments
means the argument does not exist.

That is exactly the short and long version of wm4's discourse on this
subject: "I do not have arguments, and I want the world to know it!".

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> Someone will come up with an hexdump demuxer next and call hexdump output
> a multimedia file, if we follow your views. Or an XML parser for a similarly
> arbitrary custom format they happened to devise. Or maybe oggz-dump's output.
> 
> While for the former we can tell them to go away, we wouldn't have many
> arguments against the latter two if this patch goes in. Because telling them
> we don't want their custom format after this one was committed would be
> hypocritical, biased, and a decision as arbitrary as the actual hypothetical
> format.

If these formats are useful, they should be accepted. This should be the
main criterions: usefulness and drawbacks.

Your argument seem to assume that someone will maliciously try to get a
crappy demuxer accepted that way. Seems a bit paranoid.

> Aside from the "arbitrary markup format" argument, the fact it's disabled by
> default for security reasons doesn't exactly inspire confidence.
> It would afaik be a first for an internal module that doesn't depend on
> external factors.

Disabled by default was a concession to someone (Paul, IIRC) raising the
concern of security. It seems very feeble to me: why this format and not
the many others?

If this is giving you pause, then let us review all this carefully, fuzz
it, add regression testing, and enable it by default.

> Besides, nothing even /muxes/ this format to begin with. The doxy states you
> should take the ffprobe standard output and edit it by hand or with a script
> until it looks like something this demuxer can digest.

Stefano has promised to implement a corresponding muxer (rather easy,
actually, but a bit of code duplication with ffprobe) and has also
tried to get ffprobe to output the exact format.

> Which so happens to be what every libav* user has to do for their projects.
> Write a program using the libraries' public API to read and write files.
> 
> We have plenty of times refused code people wanted to dump into libavformat
> and libavcodec because it was the easiest way for them to get what they
> needed done. There was this absolutely insane youtube-dl "demuxer" that still
> gives me the chills.
> We have also plenty of times let things go in, even with several developers
> against it. Lets try to not do it again.

This argument mixes a whole lot of different cases together. Each case
has its own specifics that should be used to judge whether it should
have been accepted or not.

Once again: usefulness and drawbacks.

Having one demuxer of this kind, as powerful as possible, is useful. Do
you not agree with that?

(Having several of them would be less useful.)

What are the drawbacks you see?

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Use the first sidx for tracks without sidx.

2016-10-31 Thread Sasi Inguva
ping.

Thanks!

On Wed, Oct 26, 2016 at 11:31 AM, Sasi Inguva  wrote:

> According to spec ISO_IEC_15444_12 "For any media stream for which no
> segment index is present, referred to as non‐indexed stream, the media
> stream associated with the first Segment Index box in the segment serves as
> a reference stream in a sense that it also describes the subsegments for
> any non‐indexed media stream."
>
> Signed-off-by: Sasi Inguva 
> ---
>  libavformat/isom.h |  1 +
>  libavformat/mov.c  | 25 ++---
>  2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index 9038057..d684502 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -179,6 +179,7 @@ typedef struct MOVStreamContext {
>  int32_t *display_matrix;
>  uint32_t format;
>
> +int has_sidx;  // If there is an sidx entry for this stream.
>  struct {
>  int use_subsamples;
>  uint8_t* auxiliary_info;
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 357d800..d9ed5a3 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4202,7 +4202,8 @@ static int mov_read_sidx(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
>  uint8_t version;
>  unsigned i, track_id;
>  AVStream *st = NULL;
> -MOVStreamContext *sc;
> +AVStream *ref_st;
> +MOVStreamContext *sc, *ref_sc;
>  MOVFragmentIndex *index = NULL;
>  MOVFragmentIndex **tmp;
>  AVRational timescale;
> @@ -4284,9 +4285,26 @@ static int mov_read_sidx(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
>
>  c->fragment_index_data = tmp;
>  c->fragment_index_data[c->fragment_index_count++] = index;
> +sc->has_sidx = 1;
> +
> +if (offset == avio_size(pb)) {
> +for (i = 0; i < c->fc->nb_streams; i++) {
> +if (c->fc->streams[i]->id == c->fragment_index_data[0]->track_id)
> {
> +ref_st = c->fc->streams[i];
> +ref_sc = ref_st->priv_data;
> +break;
> +}
> +}
> +for (i = 0; i < c->fc->nb_streams; i++) {
> +st = c->fc->streams[i];
> +sc = st->priv_data;
> +if (!sc->has_sidx) {
> +st->duration = sc->track_end =
> av_rescale(ref_st->duration, sc->time_scale, ref_sc->time_scale);
> +}
> +}
>
> -if (offset == avio_size(pb))
>  c->fragment_index_complete = 1;
> +}
>
>  return 0;
>  }
> @@ -5846,13 +5864,14 @@ static int mov_read_packet(AVFormatContext *s,
> AVPacket *pkt)
>  static int mov_seek_fragment(AVFormatContext *s, AVStream *st, int64_t
> timestamp)
>  {
>  MOVContext *mov = s->priv_data;
> +MOVStreamContext *sc = st->priv_data;
>  int i, j;
>
>  if (!mov->fragment_index_complete)
>  return 0;
>
>  for (i = 0; i < mov->fragment_index_count; i++) {
> -if (mov->fragment_index_data[i]->track_id == st->id) {
> +if (mov->fragment_index_data[i]->track_id == st->id ||
> !sc->has_sidx) {
>  MOVFragmentIndex *index = mov->fragment_index_data[i];
>  for (j = index->item_count - 1; j >= 0; j--) {
>  if (index->items[j].time <= timestamp) {
> --
> 2.8.0.rc3.226.g39d4020
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Set duration to zero if the duration is UINT_MAX

2016-10-31 Thread Sasi Inguva
ping.

Thanks!

On Wed, Oct 26, 2016 at 12:42 PM, Sasi Inguva  wrote:

> Attaching the file that it fixes.
>
>
> On Wed, Oct 26, 2016 at 12:40 PM, Sasi Inguva  wrote:
>
>> Fixes some MP4F files which have duration in mdhd set to UINT_MAX instead
>> of zero.
>>
>> Signed-off-by: Sasi Inguva 
>> ---
>>  libavformat/mov.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 357d800..245e424 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -1221,6 +1221,11 @@ static int mov_read_mdhd(MOVContext *c,
>> AVIOContext *pb, MOVAtom atom)
>>  sc->time_scale = avio_rb32(pb);
>>  st->duration = (version == 1) ? avio_rb64(pb) : avio_rb32(pb); /*
>> duration */
>>
>> +if ((version == 1 && st->duration == UINT64_MAX) ||
>> +(version != 1 && st->duration == UINT32_MAX)) {
>> +st->duration = 0;
>> +}
>> +
>>  lang = avio_rb16(pb); /* language */
>>  if (ff_mov_lang_to_iso639(lang, language))
>>  av_dict_set(&st->metadata, "language", language, 0);
>> --
>> 2.8.0.rc3.226.g39d4020
>>
>>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

2016-10-31 Thread Kevin Wheatley
I would really strongly suggest including DCI in the name at least -
though nobody else would choose to use it for anything other than the
reference calibration - most titles use a creative white different to
that of the encoding reference (one that is less green).

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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 1:54 PM, Nicolas George wrote:
> Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
>> Someone will come up with an hexdump demuxer next and call hexdump output
>> a multimedia file, if we follow your views. Or an XML parser for a similarly
>> arbitrary custom format they happened to devise. Or maybe oggz-dump's output.
>>
>> While for the former we can tell them to go away, we wouldn't have many
>> arguments against the latter two if this patch goes in. Because telling them
>> we don't want their custom format after this one was committed would be
>> hypocritical, biased, and a decision as arbitrary as the actual hypothetical
>> format.
> 
> If these formats are useful, they should be accepted. This should be the
> main criterions: usefulness and drawbacks.
> 
> Your argument seem to assume that someone will maliciously try to get a
> crappy demuxer accepted that way. Seems a bit paranoid.

It's an scenario that could happen, like it or not. I'd rather not open the
doors for it.

> 
>> Aside from the "arbitrary markup format" argument, the fact it's disabled by
>> default for security reasons doesn't exactly inspire confidence.
>> It would afaik be a first for an internal module that doesn't depend on
>> external factors.
> 
> Disabled by default was a concession to someone (Paul, IIRC) raising the
> concern of security. It seems very feeble to me: why this format and not
> the many others?

"why this format and not this other" is coincidentally the argument that
will be used after this thing gets committed to justify adding others.

> 
> If this is giving you pause, then let us review all this carefully, fuzz
> it, add regression testing, and enable it by default.
> 
>> Besides, nothing even /muxes/ this format to begin with. The doxy states you
>> should take the ffprobe standard output and edit it by hand or with a script
>> until it looks like something this demuxer can digest.
> 
> Stefano has promised to implement a corresponding muxer (rather easy,
> actually, but a bit of code duplication with ffprobe) and has also
> tried to get ffprobe to output the exact format.

If something is to create these files, it should be ffprobe itself and
not some unrelated muxer that's going to duplicate a lot of code.

> 
>> Which so happens to be what every libav* user has to do for their projects.
>> Write a program using the libraries' public API to read and write files.
>>
>> We have plenty of times refused code people wanted to dump into libavformat
>> and libavcodec because it was the easiest way for them to get what they
>> needed done. There was this absolutely insane youtube-dl "demuxer" that still
>> gives me the chills.
>> We have also plenty of times let things go in, even with several developers
>> against it. Lets try to not do it again.
> 
> This argument mixes a whole lot of different cases together. Each case
> has its own specifics that should be used to judge whether it should
> have been accepted or not.
> 
> Once again: usefulness and drawbacks.
> 
> Having one demuxer of this kind, as powerful as possible, is useful. Do
> you not agree with that?

It might, hence why i suggested this being implemented as a standalone tool
and not that you should drop the whole thing.

> 
> (Having several of them would be less useful.)
> 
> What are the drawbacks you see?

You presented an scenario and use case to dump media streams into a text
file to easily alter them, analyze them and reconstruct them. You then
declared said dumps to be an actual format so they could qualify for an
avformat module that could digest them.

There are no technical drawbacks for this demuxer. This is a concept and
design issue. It's a very specific need so far a single person or two
that's being forced into the libraries because it's the easiest way to
implement it.

This should be written as a standalone tool that uses lavf API to rebuild
the custom xml-like dump with the hex formatted packets as created by
ffprobe. Basically, write ffmpeg's own, more powerful and versatile
version of oggz-dump that uses lavf instead of libogg, meaning the dumps
can be created and rebuilt from and into any lavf supported format.

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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Josh de Kock

On 31/10/2016 14:15, Jean-Baptiste Kempf wrote:

Hi,

On Mon, 31 Oct 2016, at 07:03, wm4 wrote:

I very strongly disagree with this patch, but consider myself
over-voted.



I strongly disagree with this patch too.



I am extremely against the inclusion of this patch as well. It's 
something which has really limited use-cases and should be a standalone 
tool.


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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Michael Niedermayer
On Mon, Oct 31, 2016 at 02:41:05PM -0300, James Almer wrote:
> On 10/31/2016 1:54 PM, Nicolas George wrote:
> > Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
[...]
> > 
> >> Which so happens to be what every libav* user has to do for their projects.
> >> Write a program using the libraries' public API to read and write files.
> >>
> >> We have plenty of times refused code people wanted to dump into libavformat
> >> and libavcodec because it was the easiest way for them to get what they
> >> needed done. There was this absolutely insane youtube-dl "demuxer" that 
> >> still
> >> gives me the chills.
> >> We have also plenty of times let things go in, even with several developers
> >> against it. Lets try to not do it again.
> > 
> > This argument mixes a whole lot of different cases together. Each case
> > has its own specifics that should be used to judge whether it should
> > have been accepted or not.
> > 
> > Once again: usefulness and drawbacks.
> > 
> > Having one demuxer of this kind, as powerful as possible, is useful. Do
> > you not agree with that?
> 
> It might, hence why i suggested this being implemented as a standalone tool
> and not that you should drop the whole thing.
> 
> > 
> > (Having several of them would be less useful.)
> > 
> > What are the drawbacks you see?
> 
> You presented an scenario and use case to dump media streams into a text
> file to easily alter them, analyze them and reconstruct them. You then
> declared said dumps to be an actual format so they could qualify for an
> avformat module that could digest them.
> 
> There are no technical drawbacks for this demuxer. This is a concept and
> design issue. It's a very specific need so far a single person or two
> that's being forced into the libraries because it's the easiest way to
> implement it.
> 
> This should be written as a standalone tool that uses lavf API to rebuild
> the custom xml-like dump with the hex formatted packets as created by
> ffprobe. Basically, write ffmpeg's own, more powerful and versatile
> version of oggz-dump that uses lavf instead of libogg, meaning the dumps
> can be created and rebuilt from and into any lavf supported format.

A standalone tool would make some usecases more cumbersome

for example if you are debuging a video player which uses libavformat
and libavcodec.
You would dump a testcase into the text format by whatever means.
you now could edit this text file by any text editor or grep/sed tools
and then test if a problem still occurs to "bisect" down to the spot
in the file causing it to create a small testcase or to debug and
fix it.

The difference is with a demuxer you can directly play the text file
seek in it possibly do any test you need to.
with a standalone tool you first need to convert the text file back
into a standard media file.
The additional step may be slow if the file is large,
It may be difficult or impossibly to do if the case one wants to test
is not supported by the syntax of any binary format or one does not
know which binary format supports the combination of features one wants
to test. 

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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.


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


Re: [FFmpeg-devel] [PATCH] mov: only read e_old if there were any old streams

2016-10-31 Thread Sasi Inguva
First of all, if nb_old == 0 i.e. there are no entries in AVIndex, then
there is no point in calling mov_fix_index function at all. So instead of
doing the above , you can directly check for st->nb_index_entries > 0 at
the top of mov_fix_index and return otherwise.

Also, I don't understand how nb_old==0 can cause heap overflow. If I read
the code correctly, if nb_old==0  find_prev_closest_keyframe_index , should
return -1, which would make the function skip that edit list here

if (index

== -1) {av_log
(mov
->fc
,
AV_LOG_ERROR 
,
"Missing key frame while reordering index according to edit list\n");
  continue;}


On Sun, Oct 30, 2016 at 12:11 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:

> This fixes a heap buffer overflow.
>
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/mov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 357d800..95b546e 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -3028,7 +3028,7 @@ static void mov_fix_index(MOVContext *mov, AVStream
> *st)
>  // Audio decoders like AAC need need a decoder delay samples
> previous to the current sample,
>  // to correctly decode this frame. Hence for audio we seek to
> a frame 1 sec. before the
>  // edit_list_media_time to cover the decoder delay.
> -search_timestamp = FFMAX(search_timestamp - mov->time_scale,
> e_old[0].timestamp);
> +search_timestamp = FFMAX(search_timestamp - mov->time_scale,
> nb_old ? e_old[0].timestamp : INT64_MIN);
>  }
>
>  index = find_prev_closest_keyframe_index(st, e_old, nb_old,
> search_timestamp, 0);
> --
> 2.10.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: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> It's an scenario that could happen, like it or not. I'd rather not open the
> doors for it.

"Opening doors" sounds like a different wording for the slippery slope
fallacy.

> "why this format and not this other" is coincidentally the argument that
> will be used after this thing gets committed to justify adding others.

And the answer in this case is very easy: if the format is more useful
than harmful, it goes in.

> If something is to create these files, it should be ffprobe itself and
> not some unrelated muxer that's going to duplicate a lot of code.

I agree, that would be best.

> It might, hence why i suggested this being implemented as a standalone tool
> and not that you should drop the whole thing.

Ok, thanks.

> You presented an scenario and use case to dump media streams into a text
> file to easily alter them, analyze them and reconstruct them. You then
> declared said dumps to be an actual format so they could qualify for an
> avformat module that could digest them.
> 
> There are no technical drawbacks for this demuxer.

Again, thanks. Therefore, let us discuss the rest:

>This is a concept and
> design issue. It's a very specific need so far a single person or two
> that's being forced into the libraries because it's the easiest way to
> implement it.
> 
> This should be written as a standalone tool that uses lavf API to rebuild
> the custom xml-like dump with the hex formatted packets as created by
> ffprobe. Basically, write ffmpeg's own, more powerful and versatile
> version of oggz-dump that uses lavf instead of libogg, meaning the dumps
> can be created and rebuilt from and into any lavf supported format.

Ok. But compared with the demuxer approach, I see several drawbacks
(some of which I have already posted):

- More code, therefore more maintenance burden.

- When used for debugging (this is the main purpose of this feature),
  more code path, and therefore more room to trigger unrelated bugs that
  will waste time.

- Subject to the limitations of the intermediate format.

  This one is especially important: imagine I am trying to investigate a
  bug that happens with files with inconsistent timestamps. I can edit
  the dump to make the timestamps inconsistent. And then... I try to
  rebuild a file based on the dump, and lavf (used by the undump tool)
  forbids me to mux inconsistent timestamps. Game over.

  The whole point of this demuxer is to be able to build slightly broken
  input, because they are the cases that need debugging the most.
  Unfortunately, if you convert to an actual format, some of the
  brokenness will be fixed on the fly or flat out impossible.

- More steps for the person who is doing the debugging. Considering some
  debugging sessions will require hundreds of rounds, the difference
  will add up. Not just the generation step, but also the half hour lost
  because the developer forgot to convert after a change in the text
  file.

Now, your move:

* What benefits do you see for the separate tool approach?

* Can you negate any of these drawbacks?

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, Josh de Kock a écrit :
> I am extremely against the inclusion of this patch as well. It's something
> which has really limited use-cases and should be a standalone tool.

Duplicate argument.

Regards,

-- 
  Nicolas George


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


[FFmpeg-devel] [PATCH] vf_colorspace: Add support for film primaries

2016-10-31 Thread Vittorio Giovara
Signed-off-by: Vittorio Giovara 
---
This is the last easy picking, the remaining trc/prm require more convoluted
changes to the filter that will need more discussion.
Please CC.
Vittorio

 libavfilter/vf_colorspace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index 4265aa1..997c594 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -278,6 +278,7 @@ static const struct ColorPrimaries 
color_primaries[AVCOL_PRI_NB] = {
 [AVCOL_PRI_BT470BG]   = { WP_D65, 0.640, 0.330, 0.290, 0.600, 0.150, 
0.060,},
 [AVCOL_PRI_SMPTE170M] = { WP_D65, 0.630, 0.340, 0.310, 0.595, 0.155, 0.070 
},
 [AVCOL_PRI_SMPTE240M] = { WP_D65, 0.630, 0.340, 0.310, 0.595, 0.155, 0.070 
},
+[AVCOL_PRI_FILM]  = { WP_C,   0.681, 0.319, 0.243, 0.692, 0.145, 0.049 
},
 [AVCOL_PRI_SMPTE431]  = { WP_DCI, 0.680, 0.320, 0.265, 0.690, 0.150, 0.060 
},
 [AVCOL_PRI_SMPTE432]  = { WP_D65, 0.680, 0.320, 0.265, 0.690, 0.150, 0.060 
},
 [AVCOL_PRI_BT2020]= { WP_D65, 0.708, 0.292, 0.170, 0.797, 0.131, 0.046 
},
@@ -1084,6 +1085,7 @@ static const AVOption colorspace_options[] = {
 ENUM("bt470bg",  AVCOL_PRI_BT470BG,"prm"),
 ENUM("smpte170m",AVCOL_PRI_SMPTE170M,  "prm"),
 ENUM("smpte240m",AVCOL_PRI_SMPTE240M,  "prm"),
+ENUM("film", AVCOL_PRI_FILM,   "prm"),
 ENUM("smpte431", AVCOL_PRI_SMPTE431,   "prm"),
 ENUM("smpte432", AVCOL_PRI_SMPTE432,   "prm"),
 ENUM("bt2020",   AVCOL_PRI_BT2020, "prm"),
-- 
2.10.0

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


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Michael Niedermayer
On Mon, Oct 31, 2016 at 07:22:58PM +0100, Michael Niedermayer wrote:
> On Mon, Oct 31, 2016 at 02:41:05PM -0300, James Almer wrote:
> > On 10/31/2016 1:54 PM, Nicolas George wrote:
> > > Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> [...]
> > > 
> > >> Which so happens to be what every libav* user has to do for their 
> > >> projects.
> > >> Write a program using the libraries' public API to read and write files.
> > >>
> > >> We have plenty of times refused code people wanted to dump into 
> > >> libavformat
> > >> and libavcodec because it was the easiest way for them to get what they
> > >> needed done. There was this absolutely insane youtube-dl "demuxer" that 
> > >> still
> > >> gives me the chills.
> > >> We have also plenty of times let things go in, even with several 
> > >> developers
> > >> against it. Lets try to not do it again.
> > > 
> > > This argument mixes a whole lot of different cases together. Each case
> > > has its own specifics that should be used to judge whether it should
> > > have been accepted or not.
> > > 
> > > Once again: usefulness and drawbacks.
> > > 
> > > Having one demuxer of this kind, as powerful as possible, is useful. Do
> > > you not agree with that?
> > 
> > It might, hence why i suggested this being implemented as a standalone tool
> > and not that you should drop the whole thing.
> > 
> > > 
> > > (Having several of them would be less useful.)
> > > 
> > > What are the drawbacks you see?
> > 
> > You presented an scenario and use case to dump media streams into a text
> > file to easily alter them, analyze them and reconstruct them. You then
> > declared said dumps to be an actual format so they could qualify for an
> > avformat module that could digest them.
> > 
> > There are no technical drawbacks for this demuxer. This is a concept and
> > design issue. It's a very specific need so far a single person or two
> > that's being forced into the libraries because it's the easiest way to
> > implement it.
> > 
> > This should be written as a standalone tool that uses lavf API to rebuild
> > the custom xml-like dump with the hex formatted packets as created by
> > ffprobe. Basically, write ffmpeg's own, more powerful and versatile
> > version of oggz-dump that uses lavf instead of libogg, meaning the dumps
> > can be created and rebuilt from and into any lavf supported format.
> 
> A standalone tool would make some usecases more cumbersome
> 
> for example if you are debuging a video player which uses libavformat
> and libavcodec.
> You would dump a testcase into the text format by whatever means.
> you now could edit this text file by any text editor or grep/sed tools
> and then test if a problem still occurs to "bisect" down to the spot
> in the file causing it to create a small testcase or to debug and
> fix it.
> 
> The difference is with a demuxer you can directly play the text file
> seek in it possibly do any test you need to.
> with a standalone tool you first need to convert the text file back
> into a standard media file.
> The additional step may be slow if the file is large,
> It may be difficult or impossibly to do if the case one wants to test
> is not supported by the syntax of any binary format or one does not
> know which binary format supports the combination of features one wants
> to test. 

Another case where a standalone tool would have a problem is the
case where we dont have a muxer.

To show a specific example:
Consider a bethsoftvid file, converting it to "text" and editing it
one is stuck as we have no muxer for bethsoftvid and the video codec
in it is not supported by any other muxer



[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


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


Re: [FFmpeg-devel] [PATCH] vf_colorspace: Add support for film primaries

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

On Mon, Oct 31, 2016 at 2:43 PM, Vittorio Giovara <
vittorio.giov...@gmail.com> wrote:

> Signed-off-by: Vittorio Giovara 
> ---
> This is the last easy picking, the remaining trc/prm require more
> convoluted
> changes to the filter that will need more discussion.
> Please CC.
> Vittorio
>
>  libavfilter/vf_colorspace.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
> index 4265aa1..997c594 100644
> --- a/libavfilter/vf_colorspace.c
> +++ b/libavfilter/vf_colorspace.c
> @@ -278,6 +278,7 @@ static const struct ColorPrimaries
> color_primaries[AVCOL_PRI_NB] = {
>  [AVCOL_PRI_BT470BG]   = { WP_D65, 0.640, 0.330, 0.290, 0.600, 0.150,
> 0.060,},
>  [AVCOL_PRI_SMPTE170M] = { WP_D65, 0.630, 0.340, 0.310, 0.595, 0.155,
> 0.070 },
>  [AVCOL_PRI_SMPTE240M] = { WP_D65, 0.630, 0.340, 0.310, 0.595, 0.155,
> 0.070 },
> +[AVCOL_PRI_FILM]  = { WP_C,   0.681, 0.319, 0.243, 0.692, 0.145,
> 0.049 },
>  [AVCOL_PRI_SMPTE431]  = { WP_DCI, 0.680, 0.320, 0.265, 0.690, 0.150,
> 0.060 },
>  [AVCOL_PRI_SMPTE432]  = { WP_D65, 0.680, 0.320, 0.265, 0.690, 0.150,
> 0.060 },
>  [AVCOL_PRI_BT2020]= { WP_D65, 0.708, 0.292, 0.170, 0.797, 0.131,
> 0.046 },
> @@ -1084,6 +1085,7 @@ static const AVOption colorspace_options[] = {
>  ENUM("bt470bg",  AVCOL_PRI_BT470BG,"prm"),
>  ENUM("smpte170m",AVCOL_PRI_SMPTE170M,  "prm"),
>  ENUM("smpte240m",AVCOL_PRI_SMPTE240M,  "prm"),
> +ENUM("film", AVCOL_PRI_FILM,   "prm"),
>  ENUM("smpte431", AVCOL_PRI_SMPTE431,   "prm"),
>  ENUM("smpte432", AVCOL_PRI_SMPTE432,   "prm"),
>  ENUM("bt2020",   AVCOL_PRI_BT2020, "prm"),
> --
> 2.10.0


I can't believe you found content that uses this :D

lgtm.

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


Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

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

On Mon, Oct 31, 2016 at 11:13 AM, Vittorio Giovara <
vittorio.giov...@gmail.com> wrote:

> On Mon, Oct 31, 2016 at 7:43 AM, Ronald S. Bultje 
> wrote:
> > Hi,
> >
> > On Mon, Oct 31, 2016 at 5:50 AM, Kevin Wheatley <
> kevin.j.wheat...@gmail.com>
> > wrote:
> >>
> >> On Sun, Oct 30, 2016 at 1:18 PM, Ronald S. Bultje 
> >> wrote:
> >> > Hmm... So, the wikipedia page https://en.wikipedia.org/wiki/DCI-P3
> >> > refers
> >> > to the two whitepoints here as DCI-P3 D65 and DCI-P3 Theater. Calling
> >> > one
> >> > D65 and the other DCI seems confusing in that light (assuming the
> >> > wikipedia
> >> > page is correct). I'd call it THEATER or DCI_P3_THEATER, to
> distinguish
> >> > it
> >> > from DCI-P3 D65. Is that OK?
> >>
> >> In the industry people just call it the DCI P3 white point (or 'Urgh')
> >> it is not limited to theater usage, you might consider it the
> >> calibration white point for the reference projector, so
> >> WP_DCI_P3_REFERENCE might be better, but that is a little long.
> >>
> >> I'd go for something like WP_DCI_P3 it is not really ambiguous.
> >
> >
> > Hm... OK with me (though not ideal, but what do I know). Vittorio, OK
> also?
> > I can modify patch so you don't have to resend.
>
> I find it a little long and not less confusing than my initial WP_DCI,
> among all the alternatives I liked the THEATER one the most. If that's
> a no-go, how about we could settle for WP_PROJ maybe?


Wait, wait. Length is an issue? Really?

The only reason the other names are short is because the names of the
whitepoints are short. D65 is really just called that: D65. Likewise for
D50. This name (whatever it is :D) is simply longer.

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


Re: [FFmpeg-devel] [PATCH 3/4] V14 - SCTE-35 support in hlsenc

2016-10-31 Thread Carlos Fernandez Sanz
Hi,

Just a ping in case this can be revised and/or applied soonish - thanks.

CC'ing Carl Eugen and Thilo since I could discuss this a bit in person
at the GSoC summit. Thanks both for your time by the way.

Carlos

On Tue, Oct 18, 2016 at 5:36 PM, Carlos Fernandez Sanz
 wrote:
> From: Carlos Fernandez 
>
> Signed-off-by: Carlos Fernandez 
> ---
>  libavformat/Makefile  |   2 +-
>  libavformat/hlsenc.c  | 104 --
>  libavformat/scte_35.c | 527 
> ++
>  libavformat/scte_35.h |  86 
>  4 files changed, 697 insertions(+), 22 deletions(-)
>  create mode 100644 libavformat/scte_35.c
>  create mode 100644 libavformat/scte_35.h
>
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 5d827d31..9218606 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -205,7 +205,7 @@ OBJS-$(CONFIG_HDS_MUXER) += hdsenc.o
>  OBJS-$(CONFIG_HEVC_DEMUXER)  += hevcdec.o rawdec.o
>  OBJS-$(CONFIG_HEVC_MUXER)+= rawenc.o
>  OBJS-$(CONFIG_HLS_DEMUXER)   += hls.o
> -OBJS-$(CONFIG_HLS_MUXER) += hlsenc.o
> +OBJS-$(CONFIG_HLS_MUXER) += hlsenc.o scte_35.o
>  OBJS-$(CONFIG_HNM_DEMUXER)   += hnm.o
>  OBJS-$(CONFIG_ICO_DEMUXER)   += icodec.o
>  OBJS-$(CONFIG_ICO_MUXER) += icoenc.o
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 9ca2df7..01e3237 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -38,6 +38,7 @@
>  #include "avio_internal.h"
>  #include "internal.h"
>  #include "os_support.h"
> +#include "scte_35.h"
>
>  #define KEYSIZE 16
>  #define LINE_BUFFER_SIZE 1024
> @@ -48,6 +49,10 @@ typedef struct HLSSegment {
>  double duration; /* in seconds */
>  int64_t pos;
>  int64_t size;
> +struct scte35_event *event;
> +enum scte35_event_state event_state;
> +int adv_count;
> +int64_t start_pts;
>
>  char key_uri[LINE_BUFFER_SIZE + 1];
>  char iv_string[KEYSIZE*2 + 1];
> @@ -108,6 +113,8 @@ typedef struct HLSContext {
>  int nb_entries;
>  int discontinuity_set;
>
> +int adv_count;
> +struct scte35_interface *scte_iface;
>  HLSSegment *segments;
>  HLSSegment *last_segment;
>  HLSSegment *old_segments;
> @@ -241,6 +248,8 @@ static int hls_delete_old_segments(HLSContext *hls) {
>  av_freep(&path);
>  previous_segment = segment;
>  segment = previous_segment->next;
> +if (hls->scte_iface)
> +hls->scte_iface->unref_scte35_event(&previous_segment->event);
>  av_free(previous_segment);
>  }
>
> @@ -360,8 +369,8 @@ static int hls_mux_init(AVFormatContext *s)
>  }
>
>  /* Create a new segment and append it to the segment list */
> -static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, 
> double duration,
> -  int64_t pos, int64_t size)
> +static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, 
> double duration, int64_t pos,
> +  int64_t start_pts, struct scte35_event *event, 
> int64_t size)
>  {
>  HLSSegment *en = av_malloc(sizeof(*en));
>  const char  *filename;
> @@ -384,9 +393,20 @@ static int hls_append_segment(struct AVFormatContext *s, 
> HLSContext *hls, double
>
>  en->duration = duration;
>  en->pos  = pos;
> +en->event= event;
>  en->size = size;
> +en->start_pts  = start_pts;
>  en->next = NULL;
>
> +if (hls->scte_iface) {
> +if (hls->scte_iface->event_state == EVENT_OUT_CONT)
> +hls->adv_count++;
> +else
> +hls->adv_count = 0;
> +en->event_state = hls->scte_iface->event_state;
> +}
> +
> +
>  if (hls->key_info_file) {
>  av_strlcpy(en->key_uri, hls->key_uri, sizeof(en->key_uri));
>  av_strlcpy(en->iv_string, hls->iv_string, sizeof(en->iv_string));
> @@ -460,7 +480,7 @@ static int parse_playlist(AVFormatContext *s, const char 
> *url)
>  new_start_pos = avio_tell(hls->avf->pb);
>  hls->size = new_start_pos - hls->start_pos;
>  av_strlcpy(hls->avf->filename, line, sizeof(line));
> -ret = hls_append_segment(s, hls, hls->duration, 
> hls->start_pos, hls->size);
> +ret = hls_append_segment(s, hls, hls->duration, 
> hls->start_pos, 0, NULL, hls->size);
>  if (ret < 0)
>  goto fail;
>  hls->start_pos = new_start_pos;
> @@ -590,9 +610,19 @@ static int hls_window(AVFormatContext *s, int last)
>  avio_printf(out, "#EXT-X-PROGRAM-DATE-TIME:%s.%03d%s\n", buf0, 
> milli, buf1);
>  prog_date_time += en->duration;
>  }
> -if (hls->baseurl)
> -avio_printf(out, "%s", hls->baseurl);
> -avio_printf(out, "%s\n", en->filename);
> +if (hls->scte_iface && en->event) {
> +  

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

2016-10-31 Thread Jan Ekstrom
On Mon, Oct 31, 2016 at 5:30 PM, Nicolas George  wrote:
> Le nonidi 9 brumaire, an CCXXV, Steven Liu a écrit :
>>  I saw ffmpeg have no HDS and DASH demuxer, and all of them's format is
>> use xml, maybe this parser is a very useful parser, what about the basic
>> xml :-D
>
> The Timed Text Markup Language, a subtitle format used by Youtube and
> possibly a few others, is based on XML too.
>
> I have started working on a simple XML parser, but Michael quickly found
> a bug in my attempt to remove the recursiveness in libavfilter, and I
> consider it to be the highest priority. Therefore, I stopped shortly
> after implementing the framework API and input buffer handling.
>

Hi,

As someone who thought about doing some work on formats that require
an XML parser, while I do appreciate that you are making a "simple"
XML parser, I am not sure if this is the best way forward. XML and
thus by continuation libraries that handle XML are indeed
abominations, but I am definitely not sure if we should be NIH'ing XML
parsing. For example, namespaces are already utilized in DASH/CENC.
Maybe we should just pick one XML parsing library that seems to be the
least bad of all bad alternatives, and then standardize FFmpeg on it?
Be it libxml2, libexpat or anything else?

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


Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

2016-10-31 Thread Vittorio Giovara
On Mon, Oct 31, 2016 at 2:53 PM, Ronald S. Bultje  wrote:
> Hi,
>
> On Mon, Oct 31, 2016 at 11:13 AM, Vittorio Giovara
>  wrote:
>>
>> On Mon, Oct 31, 2016 at 7:43 AM, Ronald S. Bultje 
>> wrote:
>> > Hi,
>> >
>> > On Mon, Oct 31, 2016 at 5:50 AM, Kevin Wheatley
>> > 
>> > wrote:
>> >>
>> >> On Sun, Oct 30, 2016 at 1:18 PM, Ronald S. Bultje 
>> >> wrote:
>> >> > Hmm... So, the wikipedia page https://en.wikipedia.org/wiki/DCI-P3
>> >> > refers
>> >> > to the two whitepoints here as DCI-P3 D65 and DCI-P3 Theater. Calling
>> >> > one
>> >> > D65 and the other DCI seems confusing in that light (assuming the
>> >> > wikipedia
>> >> > page is correct). I'd call it THEATER or DCI_P3_THEATER, to
>> >> > distinguish
>> >> > it
>> >> > from DCI-P3 D65. Is that OK?
>> >>
>> >> In the industry people just call it the DCI P3 white point (or 'Urgh')
>> >> it is not limited to theater usage, you might consider it the
>> >> calibration white point for the reference projector, so
>> >> WP_DCI_P3_REFERENCE might be better, but that is a little long.
>> >>
>> >> I'd go for something like WP_DCI_P3 it is not really ambiguous.
>> >
>> >
>> > Hm... OK with me (though not ideal, but what do I know). Vittorio, OK
>> > also?
>> > I can modify patch so you don't have to resend.
>>
>> I find it a little long and not less confusing than my initial WP_DCI,
>> among all the alternatives I liked the THEATER one the most. If that's
>> a no-go, how about we could settle for WP_PROJ maybe?
>
>
> Wait, wait. Length is an issue? Really?
>
> The only reason the other names are short is because the names of the
> whitepoints are short. D65 is really just called that: D65. Likewise for
> D50. This name (whatever it is :D) is simply longer.

It's not a matter of length but a matter of descriptiveness: right now
there is only one single different whitepoint defined by DCI, so IMO
it makes sense to call it simply WP_DCI. In case DCI adds new values,
naming can be modified later. The other whitepoints could also have
longer, more descriptive names too, like WP_ILLUMINANT_C, but at the
same time the WP_C shorthand is convenient and immediate (and IMO
better suited as variable name).

At any rate, pick the one you prefer ;)
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 4/4] qsv: Merge libav implementation

2016-10-31 Thread Mark Thompson
On 31/10/16 13:50, Hendrik Leppkes wrote:
> On Wed, Oct 26, 2016 at 9:50 PM, Mark Thompson  wrote:
>> Merged as-at libav 398f015, and therefore includes outstanding
>> skipped merges 04b17ff and 130e1f1.
>>
>> All features not in libav are preserved, and no options change.
>> ---
> 
> LGTM, this should make further work on this much easier and bring it
> back to a point where its actually stable - and integrates with the
> existing HW landscape (hwcontext et al).

Applied.

Thank you to everyone who assisted with testing and reviewing this series.

- Mark

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


Re: [FFmpeg-devel] [PATCH] avcodec/qsv: remove MFX_EXTBUFF_CODING_OPTION3

2016-10-31 Thread Mark Thompson
On 18/06/16 05:33, zera...@gmail.com wrote:
> From: Kyle Schwarz 
> 
> 4th generation Intel CPUs don't support MFX_EXTBUFF_CODING_OPTION3.
> 
> This patch fixes bug #5324.

Applied with the qsv merge from libav.

Thank you!

- Mark

___
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-31 Thread Michael Niedermayer
On Mon, Oct 31, 2016 at 01:48:26PM +, Rostislav Pehlivanov wrote:
> On 31 October 2016 at 13:13, Derek Buitenhuis 
> wrote:
> 
> > On 10/27/2016 10:48 PM, Sasi Inguva wrote:
> > > gentle ping.
> > >
> > > Thanks!
> >
> > A ping from me, too.
> >
> > - Derek
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> The tests pass on my machine, pushed after Derek said it looks correct.

thx

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

Avoid a single point of failure, be that a person or equipment.


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


Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

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

On Mon, Oct 31, 2016 at 3:31 PM, Vittorio Giovara <
vittorio.giov...@gmail.com> wrote:

> On Mon, Oct 31, 2016 at 2:53 PM, Ronald S. Bultje 
> wrote:
> > Hi,
> >
> > On Mon, Oct 31, 2016 at 11:13 AM, Vittorio Giovara
> >  wrote:
> >>
> >> On Mon, Oct 31, 2016 at 7:43 AM, Ronald S. Bultje 
> >> wrote:
> >> > Hi,
> >> >
> >> > On Mon, Oct 31, 2016 at 5:50 AM, Kevin Wheatley
> >> > 
> >> > wrote:
> >> >>
> >> >> On Sun, Oct 30, 2016 at 1:18 PM, Ronald S. Bultje <
> rsbul...@gmail.com>
> >> >> wrote:
> >> >> > Hmm... So, the wikipedia page https://en.wikipedia.org/wiki/DCI-P3
> >> >> > refers
> >> >> > to the two whitepoints here as DCI-P3 D65 and DCI-P3 Theater.
> Calling
> >> >> > one
> >> >> > D65 and the other DCI seems confusing in that light (assuming the
> >> >> > wikipedia
> >> >> > page is correct). I'd call it THEATER or DCI_P3_THEATER, to
> >> >> > distinguish
> >> >> > it
> >> >> > from DCI-P3 D65. Is that OK?
> >> >>
> >> >> In the industry people just call it the DCI P3 white point (or
> 'Urgh')
> >> >> it is not limited to theater usage, you might consider it the
> >> >> calibration white point for the reference projector, so
> >> >> WP_DCI_P3_REFERENCE might be better, but that is a little long.
> >> >>
> >> >> I'd go for something like WP_DCI_P3 it is not really ambiguous.
> >> >
> >> >
> >> > Hm... OK with me (though not ideal, but what do I know). Vittorio, OK
> >> > also?
> >> > I can modify patch so you don't have to resend.
> >>
> >> I find it a little long and not less confusing than my initial WP_DCI,
> >> among all the alternatives I liked the THEATER one the most. If that's
> >> a no-go, how about we could settle for WP_PROJ maybe?
> >
> >
> > Wait, wait. Length is an issue? Really?
> >
> > The only reason the other names are short is because the names of the
> > whitepoints are short. D65 is really just called that: D65. Likewise for
> > D50. This name (whatever it is :D) is simply longer.
>
> It's not a matter of length but a matter of descriptiveness: right now
> there is only one single different whitepoint defined by DCI, so IMO
> it makes sense to call it simply WP_DCI. In case DCI adds new values,
> naming can be modified later. The other whitepoints could also have
> longer, more descriptive names too, like WP_ILLUMINANT_C, but at the
> same time the WP_C shorthand is convenient and immediate (and IMO
> better suited as variable name).


That's actually a good point. I'm not sure if C is better than
ILLUMINANT_C... WDYT? I guess you're sticking to the "shorter is better"? :)

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


Re: [FFmpeg-devel] [PATCH 1/3] vf_colorspace: Add support for iec61966-2.4 (xvYCC) transfer

2016-10-31 Thread Michael Niedermayer
On Sun, Oct 30, 2016 at 09:14:47AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Sun, Oct 30, 2016 at 3:07 AM, Vittorio Giovara <
> vittorio.giov...@gmail.com> wrote:
> 
> > Signed-off-by: Vittorio Giovara 
> > ---
> > As described in http://www.color.org/chardata/rgb/xvycc.xalter
> > Please CC.
> > Vittorio
> >
> >  libavfilter/vf_colorspace.c | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> 
> OK.

applied

thx

PS: the other patches seemed not to apply cleanly so i applied just
this one.

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

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.


signature.asc
Description: Digital signature
___
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-31 Thread Zhenni Huang
On Mon, Oct 24, 2016 at 4:48 PM, Carl Eugen Hoyos 
wrote:

> 2016-10-25 1:23 GMT+02:00 Zhenni Huang  peg.org>:
>
> > 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.
>
> Yes, this is intended: If you don't want to read invalid mov files, it
> seems logical that you don't want to read other invalid files.
>
> Or do you have another use case?
>
> > It is preferable if we can have control of whether to use
> > moov in free with one separate flag.
>
> I disagree.
>
> Please do not top-post here, Carl Eugen
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Thanks Carl, I think setting strict_std_compliance is fine. Could please
you apply the change?

Sorry about the top-post, Zhenni
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] vf_colorspace: Add support for smpte 431/432 (dci/display p3) primaries

2016-10-31 Thread Vittorio Giovara
On Mon, Oct 31, 2016 at 4:08 PM, Ronald S. Bultje  wrote:
> Hi,
>
> On Mon, Oct 31, 2016 at 3:31 PM, Vittorio Giovara
>  wrote:
>>
>> On Mon, Oct 31, 2016 at 2:53 PM, Ronald S. Bultje 
>> wrote:
>> > Hi,
>> >
>> > On Mon, Oct 31, 2016 at 11:13 AM, Vittorio Giovara
>> >  wrote:
>> >>
>> >> On Mon, Oct 31, 2016 at 7:43 AM, Ronald S. Bultje 
>> >> wrote:
>> >> > Hi,
>> >> >
>> >> > On Mon, Oct 31, 2016 at 5:50 AM, Kevin Wheatley
>> >> > 
>> >> > wrote:
>> >> >>
>> >> >> On Sun, Oct 30, 2016 at 1:18 PM, Ronald S. Bultje
>> >> >> 
>> >> >> wrote:
>> >> >> > Hmm... So, the wikipedia page https://en.wikipedia.org/wiki/DCI-P3
>> >> >> > refers
>> >> >> > to the two whitepoints here as DCI-P3 D65 and DCI-P3 Theater.
>> >> >> > Calling
>> >> >> > one
>> >> >> > D65 and the other DCI seems confusing in that light (assuming the
>> >> >> > wikipedia
>> >> >> > page is correct). I'd call it THEATER or DCI_P3_THEATER, to
>> >> >> > distinguish
>> >> >> > it
>> >> >> > from DCI-P3 D65. Is that OK?
>> >> >>
>> >> >> In the industry people just call it the DCI P3 white point (or
>> >> >> 'Urgh')
>> >> >> it is not limited to theater usage, you might consider it the
>> >> >> calibration white point for the reference projector, so
>> >> >> WP_DCI_P3_REFERENCE might be better, but that is a little long.
>> >> >>
>> >> >> I'd go for something like WP_DCI_P3 it is not really ambiguous.
>> >> >
>> >> >
>> >> > Hm... OK with me (though not ideal, but what do I know). Vittorio, OK
>> >> > also?
>> >> > I can modify patch so you don't have to resend.
>> >>
>> >> I find it a little long and not less confusing than my initial WP_DCI,
>> >> among all the alternatives I liked the THEATER one the most. If that's
>> >> a no-go, how about we could settle for WP_PROJ maybe?
>> >
>> >
>> > Wait, wait. Length is an issue? Really?
>> >
>> > The only reason the other names are short is because the names of the
>> > whitepoints are short. D65 is really just called that: D65. Likewise for
>> > D50. This name (whatever it is :D) is simply longer.
>>
>> It's not a matter of length but a matter of descriptiveness: right now
>> there is only one single different whitepoint defined by DCI, so IMO
>> it makes sense to call it simply WP_DCI. In case DCI adds new values,
>> naming can be modified later. The other whitepoints could also have
>> longer, more descriptive names too, like WP_ILLUMINANT_C, but at the
>> same time the WP_C shorthand is convenient and immediate (and IMO
>> better suited as variable name).
>
>
> That's actually a good point. I'm not sure if C is better than
> ILLUMINANT_C... WDYT? I guess you're sticking to the "shorter is better"? :)

In this case, yes, shorter is better, in my opinion.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] pixblockdsp: disable altivec optimizations on ppc64be

2016-10-31 Thread Andreas Cadhalpun
The checkasm test fails, see trac ticket 5508.

Also, the following tests fail due to this:
fate-vsynth1-dnxhd-2k-hr-hq fate-vsynth1-dnxhd-edge1-hr
fate-vsynth1-dnxhd-edge2-hr fate-vsynth1-dnxhd-edge3-hr
fate-vsynth1-dnxhd-hr-sq-mov fate-vsynth1-dnxhd-hr-hq-mov
fate-vsynth2-dnxhd-2k-hr-hq fate-vsynth2-dnxhd-edge1-hr
fate-vsynth2-dnxhd-edge2-hr fate-vsynth2-dnxhd-edge3-hr
fate-vsynth2-dnxhd-hr-sq-mov fate-vsynth2-dnxhd-hr-hq-mov
fate-vsynth3-dnxhd-2k-hr-hq fate-vsynth3-dnxhd-edge1-hr
fate-vsynth3-dnxhd-edge2-hr fate-vsynth3-dnxhd-edge3-hr
fate-vsynth3-dnxhd-hr-sq-mov fate-vsynth3-dnxhd-hr-hq-mov

Signed-off-by: Andreas Cadhalpun 
---

Just disabling the checkasm_check_pixblockdsp test for ppc64be,
as was done in commit e5d434 for release/3.1, does not make much sense,
as the altivec functions actually don't work...

---
 libavcodec/ppc/pixblockdsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/ppc/pixblockdsp.c b/libavcodec/ppc/pixblockdsp.c
index 84aa562..7822eb0 100644
--- a/libavcodec/ppc/pixblockdsp.c
+++ b/libavcodec/ppc/pixblockdsp.c
@@ -266,7 +266,7 @@ av_cold void ff_pixblockdsp_init_ppc(PixblockDSPContext *c,
  AVCodecContext *avctx,
  unsigned high_bit_depth)
 {
-#if HAVE_ALTIVEC
+#if HAVE_ALTIVEC && !(ARCH_PPC64 && HAVE_BIGENDIAN)
 if (!PPC_ALTIVEC(av_get_cpu_flags()))
 return;
 
-- 
2.10.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] interplayacm: increase bitstream buffer size by AV_INPUT_BUFFER_PADDING_SIZE

2016-10-31 Thread Andreas Cadhalpun
On 31.10.2016 08:33, Paul B Mahol wrote:
> On 10/30/16, Andreas Cadhalpun  wrote:
>> On 30.10.2016 22:18, Paul B Mahol wrote:
>>> On 10/30/16, Andreas Cadhalpun  wrote:
 This fixes out-of-bounds reads by the bitstream reader.

 Signed-off-by: Andreas Cadhalpun 
 ---
  libavcodec/interplayacm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/libavcodec/interplayacm.c b/libavcodec/interplayacm.c
 index 0486e00..f4a3446 100644
 --- a/libavcodec/interplayacm.c
 +++ b/libavcodec/interplayacm.c
 @@ -72,7 +72,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
  s->block   = av_calloc(s->block_len, sizeof(int));
  s->wrapbuf = av_calloc(s->wrapbuf_len, sizeof(int));
  s->ampbuf  = av_calloc(0x1, sizeof(int));
 -s->bitstream = av_calloc(s->max_framesize, sizeof(*s->bitstream));
 +s->bitstream = av_calloc(s->max_framesize +
 AV_INPUT_BUFFER_PADDING_SIZE / sizeof(*s->bitstream) + 1,
>>>
>>> How did you came up with this fix?
>>> Little background would help.
>>
>> The out-of-bounds read happens in get_bits called from linear.
>> The buffer passed to init_get_bits8 is &s->bitstream[s->bitstream_index].
>> The get_bits documentation says:
>> /**
>>  * Initialize GetBitContext.
>>  * @param buffer bitstream buffer, must be AV_INPUT_BUFFER_PADDING_SIZE
>> bytes
>>  *larger than the actual read bits because some optimized bitstream
>>  *readers read 32 or 64 bit at once and could read over the end
>>  * @param byte_size the size of the buffer in bytes
>>  * @return 0 on success, AVERROR_INVALIDDATA if the buffer_size would
>> overflow.
>>  */
>> static inline int init_get_bits8(GetBitContext *s, const uint8_t *buffer,
>>  int byte_size)
>>
>> Increasing the buffer size fixed the problem, so the case seems quite clear.
>>
>> Best regards,
>> Andreas
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> 
> ok

Pushed.

Best regards,
Andreas

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


Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Set duration to zero if the duration is UINT_MAX

2016-10-31 Thread Yusuke Nakamura
2016-10-27 4:40 GMT+09:00 Sasi Inguva :

> Fixes some MP4F files which have duration in mdhd set to UINT_MAX instead
> of zero.
>

What does this patch fix?


>
> Signed-off-by: Sasi Inguva 
> ---
>  libavformat/mov.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 357d800..245e424 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1221,6 +1221,11 @@ static int mov_read_mdhd(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
>  sc->time_scale = avio_rb32(pb);
>  st->duration = (version == 1) ? avio_rb64(pb) : avio_rb32(pb); /*
> duration */
>
> +if ((version == 1 && st->duration == UINT64_MAX) ||
> +(version != 1 && st->duration == UINT32_MAX)) {
> +st->duration = 0;
> +}
> +
>  lang = avio_rb16(pb); /* language */
>  if (ff_mov_lang_to_iso639(lang, language))
>  av_dict_set(&st->metadata, "language", language, 0);
> --
> 2.8.0.rc3.226.g39d4020
>
> ___
> 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] mov: only read e_old if there were any old streams

2016-10-31 Thread Andreas Cadhalpun
On 31.10.2016 19:20, Sasi Inguva wrote:
> First of all, if nb_old == 0 i.e. there are no entries in AVIndex, then
> there is no point in calling mov_fix_index function at all. So instead of
> doing the above , you can directly check for st->nb_index_entries > 0 at
> the top of mov_fix_index and return otherwise.

OK, patch doing that is attached.

> Also, I don't understand how nb_old==0 can cause heap overflow. If I read
> the code correctly, if nb_old==0  find_prev_closest_keyframe_index , should
> return -1, which would make the function skip that edit list here
> 
> if (index == -1) {
>av_log(mov>->fc, AV_LOG_ERROR, "Missing key frame while reordering 
> index according to edit list\n");
>   continue;
>}

This checks is four lines below the heap buffer overflow, which is obviously 
too late.

Best regards,
Andreas
>From 634682d0628d02a2941140800e901611bfee2d0c Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun 
Date: Tue, 1 Nov 2016 01:05:01 +0100
Subject: [PATCH] mov: immediately return from mov_fix_index without old index
 entries

If there are no index entries, e_old = st->index_entries is only one
byte large, since it was created by av_realloc called with size 0.

Thus accessing e_old[0].timestamp causes a heap buffer overflow.

Signed-off-by: Andreas Cadhalpun 
---
 libavformat/mov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 414007e..7614632 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2961,7 +2961,7 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
 int first_non_zero_audio_edit = -1;
 int packet_skip_samples = 0;
 
-if (!msc->elst_data || msc->elst_count <= 0) {
+if (!msc->elst_data || msc->elst_count <= 0 || nb_old <= 0) {
 return;
 }
 // Clean AVStream from traces of old index
-- 
2.10.1

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


Re: [FFmpeg-devel] [PATCH] pixblockdsp: disable altivec optimizations on ppc64be

2016-10-31 Thread Michael Niedermayer
On Tue, Nov 01, 2016 at 12:27:00AM +0100, Andreas Cadhalpun wrote:
> The checkasm test fails, see trac ticket 5508.
> 
> Also, the following tests fail due to this:
> fate-vsynth1-dnxhd-2k-hr-hq fate-vsynth1-dnxhd-edge1-hr
> fate-vsynth1-dnxhd-edge2-hr fate-vsynth1-dnxhd-edge3-hr
> fate-vsynth1-dnxhd-hr-sq-mov fate-vsynth1-dnxhd-hr-hq-mov
> fate-vsynth2-dnxhd-2k-hr-hq fate-vsynth2-dnxhd-edge1-hr
> fate-vsynth2-dnxhd-edge2-hr fate-vsynth2-dnxhd-edge3-hr
> fate-vsynth2-dnxhd-hr-sq-mov fate-vsynth2-dnxhd-hr-hq-mov
> fate-vsynth3-dnxhd-2k-hr-hq fate-vsynth3-dnxhd-edge1-hr
> fate-vsynth3-dnxhd-edge2-hr fate-vsynth3-dnxhd-edge3-hr
> fate-vsynth3-dnxhd-hr-sq-mov fate-vsynth3-dnxhd-hr-hq-mov
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
> 
> Just disabling the checkasm_check_pixblockdsp test for ppc64be,
> as was done in commit e5d434 for release/3.1, does not make much sense,
> as the altivec functions actually don't work...

has this been tested with actual hw or an emulator ?
is this a regression ? if so since when ?

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

Avoid a single point of failure, be that a person or equipment.


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


Re: [FFmpeg-devel] [PATCH 1/2] Add experimental muxing support for FLAC in ISO BMFF (MP4).

2016-10-31 Thread Matthew Gregan
At 2016-10-31T11:04:16-0300, James Almer wrote:
> > +static int mov_write_dfla_tag(AVIOContext *pb, MOVTrack *track)
> > +{
> > +const size_t FLAC_STREAMINFO_SIZE = 34;
> > +int64_t pos = avio_tell(pb);
> > +avio_wb32(pb, 0);
> > +ffio_wfourcc(pb, "dfLa");
> > +avio_w8(pb, 0); /* version */
> > +avio_wb24(pb, 0); /* flags */
> > +
> > +/* Expect the encoder to pass a METADATA_BLOCK_TYPE_STREAMINFO. */
> 
> The encoder may (or most likely will) also pass an updated streaminfo as
> packet side data.  See the flac muxer, you'll have to do the same here.

Thanks.  It looks like libavcodec/flacenc.c#flac_encode_frame updates the
extradata at the same time it emits the packet side data, so I'm slightly
confused why this is needed.

Seems to be easy to handle by extending the existing case for
AV_CODEC_ID_MP4ALS in mov_write_single_packet.  That assumes pkt->size == 0,
which seems to be true when the packet side data is emitted in
flac_encode_frame.

Updated patch attached.
>From ba197e6e9df3916f7dd433736c580b27f3c6ca75 Mon Sep 17 00:00:00 2001
Message-Id: 
From: Matthew Gregan 
Date: Thu, 20 Oct 2016 17:28:11 +1300
Subject: [PATCH 1/2] Add experimental muxing support for FLAC in ISO BMFF
 (MP4).

Based on the draft spec at https://git.xiph.org/?p=flac.git;a=blob;f=doc/isoflac.txt

'-strict -2' is required to create files in this format.

Signed-off-by: Matthew Gregan 
---
 libavformat/isom.c   |  2 ++
 libavformat/movenc.c | 46 +++---
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/libavformat/isom.c b/libavformat/isom.c
index ab79e22..aacbe43 100644
--- a/libavformat/isom.c
+++ b/libavformat/isom.c
@@ -60,6 +60,7 @@ const AVCodecTag ff_mp4_obj_type[] = {
 { AV_CODEC_ID_EAC3, 0xA6 },
 { AV_CODEC_ID_DTS , 0xA9 }, /* mp4ra.org */
 { AV_CODEC_ID_VP9 , 0xC0 }, /* nonstandard, update when there is a standard value */
+{ AV_CODEC_ID_FLAC, 0xC1 }, /* nonstandard, update when there is a standard value */
 { AV_CODEC_ID_TSCC2   , 0xD0 }, /* nonstandard, camtasia uses it */
 { AV_CODEC_ID_VORBIS  , 0xDD }, /* nonstandard, gpac uses it */
 { AV_CODEC_ID_DVD_SUBTITLE, 0xE0 }, /* nonstandard, see unsupported-embedded-subs-2.mp4 */
@@ -345,6 +346,7 @@ const AVCodecTag ff_codec_movaudio_tags[] = {
 { AV_CODEC_ID_WMAV2,   MKTAG('W', 'M', 'A', '2') },
 { AV_CODEC_ID_EVRC,MKTAG('s', 'e', 'v', 'c') }, /* 3GPP2 */
 { AV_CODEC_ID_SMV, MKTAG('s', 's', 'm', 'v') }, /* 3GPP2 */
+{ AV_CODEC_ID_FLAC,MKTAG('f', 'L', 'a', 'C') }, /* nonstandard */
 { AV_CODEC_ID_NONE, 0 },
 };
 
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 6228192..0a6ffb0 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -654,6 +654,27 @@ static int mov_write_wfex_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
 return update_size(pb, pos);
 }
 
+static int mov_write_dfla_tag(AVIOContext *pb, MOVTrack *track)
+{
+const size_t FLAC_STREAMINFO_SIZE = 34;
+int64_t pos = avio_tell(pb);
+avio_wb32(pb, 0);
+ffio_wfourcc(pb, "dfLa");
+avio_w8(pb, 0); /* version */
+avio_wb24(pb, 0); /* flags */
+
+/* Expect the encoder to pass a METADATA_BLOCK_TYPE_STREAMINFO. */
+if (track->par->extradata_size != FLAC_STREAMINFO_SIZE)
+  return AVERROR_INVALIDDATA;
+
+/* TODO: Write other METADATA_BLOCK_TYPEs if the encoder makes them available. */
+avio_w8(pb, 1 << 7 | 0); /* LastMetadataBlockFlag << 7 | BlockType (STREAMINFO) */
+avio_wb24(pb, track->par->extradata_size); /* Length */
+avio_write(pb, track->par->extradata, track->par->extradata_size); /* BlockData[Length] */
+
+return update_size(pb, pos);
+}
+
 static int mov_write_chan_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
 {
 uint32_t layout_tag, bitmap;
@@ -963,8 +984,13 @@ static int mov_write_audio_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContex
 avio_wb16(pb, 16);
 avio_wb16(pb, track->audio_vbr ? -2 : 0); /* compression ID */
 } else { /* reserved for mp4/3gp */
-avio_wb16(pb, 2);
-avio_wb16(pb, 16);
+if (track->par->codec_id == AV_CODEC_ID_FLAC) {
+avio_wb16(pb, track->par->channels);
+avio_wb16(pb, av_get_bytes_per_sample(track->par->format) * 8);
+} else {
+avio_wb16(pb, 2);
+avio_wb16(pb, 16);
+}
 avio_wb16(pb, 0);
 }
 
@@ -1009,6 +1035,8 @@ static int mov_write_audio_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContex
 mov_write_extradata_tag(pb, track);
 else if (track->par->codec_id == AV_CODEC_ID_WMAPRO)
 mov_write_wfex_tag(s, pb, track);
+else if (track->par->codec_id == AV_CODEC_ID_FLAC)
+mov_write_dfla_tag(pb, track);
 else if (track->vos_len > 0)
 mov_write_glbl

Re: [FFmpeg-devel] [PATCH] pixblockdsp: disable altivec optimizations on ppc64be

2016-10-31 Thread Andreas Cadhalpun
On 01.11.2016 02:04, Michael Niedermayer wrote:
> On Tue, Nov 01, 2016 at 12:27:00AM +0100, Andreas Cadhalpun wrote:
>> The checkasm test fails, see trac ticket 5508.
>>
>> Also, the following tests fail due to this:
>> fate-vsynth1-dnxhd-2k-hr-hq fate-vsynth1-dnxhd-edge1-hr
>> fate-vsynth1-dnxhd-edge2-hr fate-vsynth1-dnxhd-edge3-hr
>> fate-vsynth1-dnxhd-hr-sq-mov fate-vsynth1-dnxhd-hr-hq-mov
>> fate-vsynth2-dnxhd-2k-hr-hq fate-vsynth2-dnxhd-edge1-hr
>> fate-vsynth2-dnxhd-edge2-hr fate-vsynth2-dnxhd-edge3-hr
>> fate-vsynth2-dnxhd-hr-sq-mov fate-vsynth2-dnxhd-hr-hq-mov
>> fate-vsynth3-dnxhd-2k-hr-hq fate-vsynth3-dnxhd-edge1-hr
>> fate-vsynth3-dnxhd-edge2-hr fate-vsynth3-dnxhd-edge3-hr
>> fate-vsynth3-dnxhd-hr-sq-mov fate-vsynth3-dnxhd-hr-hq-mov
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>
>> Just disabling the checkasm_check_pixblockdsp test for ppc64be,
>> as was done in commit e5d434 for release/3.1, does not make much sense,
>> as the altivec functions actually don't work...
> 
> has this been tested with actual hw or an emulator ?

The tests failed on the Debian buildd [1] and I could reproduce the exact
same failures with qemu. The patch is only tested with qemu.

> is this a regression ? if so since when ?

Well, the commit e5d434 was never on master, but in release/3.1.
And the failing dnxhd tests are new:
eb5f4b1 tests/fate/vcodec: add dnxhr mov tests
44ac2b9 tests/fate/vcodec: add dnxhr edge tests
6108cb2 tests/fate: add dnxhr encoding tests

So this altivec code probably never worked on ppc64be, it was just not
tested.

Best regards,
Andreas


1: 
https://buildd.debian.org/status/fetch.php?pkg=ffmpeg&arch=ppc64&ver=7%3A3.2-1&stamp=1477927610

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


[FFmpeg-devel] [PATCH] avcodec/dnxhdenc: Fix alignment of edge_buf*

2016-10-31 Thread Michael Niedermayer
Signed-off-by: Michael Niedermayer 
---
 libavcodec/dnxhdenc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/dnxhdenc.h b/libavcodec/dnxhdenc.h
index 91ef6d0..eb9da12 100644
--- a/libavcodec/dnxhdenc.h
+++ b/libavcodec/dnxhdenc.h
@@ -71,8 +71,8 @@ typedef struct DNXHDEncContext {
 int intra_quant_bias;
 
 DECLARE_ALIGNED(16, int16_t, blocks)[8][64];
-uint8_t edge_buf_y[256];
-uint8_t edge_buf_uv[2][128];
+DECLARE_ALIGNED(16, uint8_t, edge_buf_y)[256];
+DECLARE_ALIGNED(16, uint8_t, edge_buf_uv)[2][128];
 
 int  (*qmatrix_c) [64];
 int  (*qmatrix_l) [64];
-- 
2.10.1

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


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

2016-10-31 Thread Steven Liu
2016-11-01 3:16 GMT+08:00 Jan Ekstrom :

> On Mon, Oct 31, 2016 at 5:30 PM, Nicolas George  wrote:
> > Le nonidi 9 brumaire, an CCXXV, Steven Liu a écrit :
> >>  I saw ffmpeg have no HDS and DASH demuxer, and all of them's
> format is
> >> use xml, maybe this parser is a very useful parser, what about the basic
> >> xml :-D
> >
> > The Timed Text Markup Language, a subtitle format used by Youtube and
> > possibly a few others, is based on XML too.
> >
> > I have started working on a simple XML parser, but Michael quickly found
> > a bug in my attempt to remove the recursiveness in libavfilter, and I
> > consider it to be the highest priority. Therefore, I stopped shortly
> > after implementing the framework API and input buffer handling.
> >
>
> Hi,
>
> As someone who thought about doing some work on formats that require
> an XML parser, while I do appreciate that you are making a "simple"
> XML parser, I am not sure if this is the best way forward. XML and
> thus by continuation libraries that handle XML are indeed
> abominations, but I am definitely not sure if we should be NIH'ing XML
> parsing. For example, namespaces are already utilized in DASH/CENC.
> Maybe we should just pick one XML parsing library that seems to be the
> least bad of all bad alternatives, and then standardize FFmpeg on it?
> Be it libxml2, libexpat or anything else?
>
> Best regards,
> Jan
>

Hi Jan Ekstrom,
I cannot sure if i misunderstand you, but i think FFmpeg need a basic
XML parser,
because the FFmpeg have the muxer of DASH/HDS and so on but have no
demuxer, and not only FFmpeg have the muxer to the DASH/ HDS and so on.
There have many tools can muxing the format.
So the XML format is not unify , so need a basic XML parser to paring the
simple XML format at first.
of course, use the third part libxml2 or libexpat is a way to parse the
XML, but i think FFmpeg need a parser by itself, not depend on third part
and use --enable-, --disable- to decide the DASH/HDS demuxer enable
or not perhaps not a better way.

> ___
> 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] avformat/apngenc: use the stream parameters extradata if no updated one is made available

2016-10-31 Thread James Almer
Fixes remuxing apng streams coming from the apng demuxer.
This is a regression since 97792e85c338d129342f5812e2a52048373e57d6.

Signed-off-by: James Almer 
---
 libavformat/apngenc.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/libavformat/apngenc.c b/libavformat/apngenc.c
index e5e8aa9..2b88dcc 100644
--- a/libavformat/apngenc.c
+++ b/libavformat/apngenc.c
@@ -101,6 +101,13 @@ static int apng_write_header(AVFormatContext 
*format_context)
 avio_wb64(format_context->pb, PNGSIG);
 // Remaining headers are written when they are copied from the encoder
 
+apng->extra_data = 
av_mallocz(format_context->streams[0]->codecpar->extradata_size + 
AV_INPUT_BUFFER_PADDING_SIZE);
+if (!apng->extra_data)
+return AVERROR(ENOMEM);
+apng->extra_data_size =  
format_context->streams[0]->codecpar->extradata_size;
+memcpy(apng->extra_data, format_context->streams[0]->codecpar->extradata,
+   format_context->streams[0]->codecpar->extradata_size);
+
 return 0;
 }
 
-- 
2.10.1

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


Re: [FFmpeg-devel] [PATCH] avformat/apngenc: use the stream parameters extradata if no updated one is made available

2016-10-31 Thread James Almer
On 10/31/2016 11:32 PM, James Almer wrote:
> Fixes remuxing apng streams coming from the apng demuxer.
> This is a regression since 97792e85c338d129342f5812e2a52048373e57d6.

Should be 940b8908b94404a65f9f55e33efb4ccc6c81383c, sorry.

> 
> Signed-off-by: James Almer 
> ---
>  libavformat/apngenc.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libavformat/apngenc.c b/libavformat/apngenc.c
> index e5e8aa9..2b88dcc 100644
> --- a/libavformat/apngenc.c
> +++ b/libavformat/apngenc.c
> @@ -101,6 +101,13 @@ static int apng_write_header(AVFormatContext 
> *format_context)
>  avio_wb64(format_context->pb, PNGSIG);
>  // Remaining headers are written when they are copied from the encoder
>  
> +apng->extra_data = 
> av_mallocz(format_context->streams[0]->codecpar->extradata_size + 
> AV_INPUT_BUFFER_PADDING_SIZE);
> +if (!apng->extra_data)
> +return AVERROR(ENOMEM);
> +apng->extra_data_size =  
> format_context->streams[0]->codecpar->extradata_size;
> +memcpy(apng->extra_data, format_context->streams[0]->codecpar->extradata,
> +   format_context->streams[0]->codecpar->extradata_size);
> +
>  return 0;
>  }
>  
> 

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


Re: [FFmpeg-devel] [PATCH 1/2] Add experimental muxing support for FLAC in ISO BMFF (MP4).

2016-10-31 Thread James Almer
On 10/31/2016 10:11 PM, Matthew Gregan wrote:
> At 2016-10-31T11:04:16-0300, James Almer wrote:
>>> +static int mov_write_dfla_tag(AVIOContext *pb, MOVTrack *track)
>>> +{
>>> +const size_t FLAC_STREAMINFO_SIZE = 34;
>>> +int64_t pos = avio_tell(pb);
>>> +avio_wb32(pb, 0);
>>> +ffio_wfourcc(pb, "dfLa");
>>> +avio_w8(pb, 0); /* version */
>>> +avio_wb24(pb, 0); /* flags */
>>> +
>>> +/* Expect the encoder to pass a METADATA_BLOCK_TYPE_STREAMINFO. */
>>
>> The encoder may (or most likely will) also pass an updated streaminfo as
>> packet side data.  See the flac muxer, you'll have to do the same here.
> 
> Thanks.  It looks like libavcodec/flacenc.c#flac_encode_frame updates the
> extradata at the same time it emits the packet side data, so I'm slightly
> confused why this is needed.

The muxer gets the extradata as created by the encoder only during init().
Even if the encoder changes it in any way afterwards, it will never make
it to the muxer. That's why packet side data is used for this.

The reason the encoder used the AVCodecContext extradata to store the
updated Streaminfo before creating the packet side data is most likely
because it was an already allocated buffer. Technically, it's not the
"proper" thing to do, but it also doesn't seem to make a difference.

> 
> Seems to be easy to handle by extending the existing case for
> AV_CODEC_ID_MP4ALS in mov_write_single_packet.  That assumes pkt->size == 0,
> which seems to be true when the packet side data is emitted in
> flac_encode_frame.
> 
> Updated patch attached.

Looks like it works now.

No comments about the actual implementation or the spec. I'll let someone
more familiar with mp4/mov do that instead.

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


Re: [FFmpeg-devel] [PATCH] avformat/apngenc: use the stream parameters extradata if no updated one is made available

2016-10-31 Thread James Almer
On 10/31/2016 11:32 PM, James Almer wrote:
> Fixes remuxing apng streams coming from the apng demuxer.
> This is a regression since 97792e85c338d129342f5812e2a52048373e57d6.
> 
> Signed-off-by: James Almer 
> ---
>  libavformat/apngenc.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libavformat/apngenc.c b/libavformat/apngenc.c
> index e5e8aa9..2b88dcc 100644
> --- a/libavformat/apngenc.c
> +++ b/libavformat/apngenc.c
> @@ -101,6 +101,13 @@ static int apng_write_header(AVFormatContext 
> *format_context)
>  avio_wb64(format_context->pb, PNGSIG);
>  // Remaining headers are written when they are copied from the encoder
>  
> +apng->extra_data = 
> av_mallocz(format_context->streams[0]->codecpar->extradata_size + 
> AV_INPUT_BUFFER_PADDING_SIZE);
> +if (!apng->extra_data)
> +return AVERROR(ENOMEM);
> +apng->extra_data_size =  
> format_context->streams[0]->codecpar->extradata_size;
> +memcpy(apng->extra_data, format_context->streams[0]->codecpar->extradata,
> +   format_context->streams[0]->codecpar->extradata_size);
> +

Alternatively we could just go back to the first version of Andreas' patch,
where the codecpar extradata was overwritten by the updated one from the
packet side data, and get rid of the private context buffer.

I assumed keeping the codecpar extradata intact was the correct behavior,
based on the avcodec.h documentation and since AVCodecParameters is an
intermediary struct meant to pass parameters between de/muxers and
de/encoders, but it seems the mov and flv muxers just overwrite it. I'm not
sure if it makes a difference at all.

Any opinions?

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


Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Set duration to zero if the duration is UINT_MAX

2016-10-31 Thread Sasi Inguva
For MP4F files , stream duration is updated by the sum of duration of
moof's
http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/mov.c;h=414007e7aa128aa66e07395b42c0cd4b369b3146;hb=HEAD#l4193
 , only if the duration in the header is less than that. Normally duration
field in header is 0, so this is ok. But some MP4F files have it set to
UINT_MAX , like the example I have uploaded.

On Mon, Oct 31, 2016 at 5:16 PM, Yusuke Nakamura <
muken.the.vfrman...@gmail.com> wrote:

> 2016-10-27 4:40 GMT+09:00 Sasi Inguva :
>
> > Fixes some MP4F files which have duration in mdhd set to UINT_MAX instead
> > of zero.
> >
>
> What does this patch fix?
>
>
> >
> > Signed-off-by: Sasi Inguva 
> > ---
> >  libavformat/mov.c | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 357d800..245e424 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -1221,6 +1221,11 @@ static int mov_read_mdhd(MOVContext *c,
> AVIOContext
> > *pb, MOVAtom atom)
> >  sc->time_scale = avio_rb32(pb);
> >  st->duration = (version == 1) ? avio_rb64(pb) : avio_rb32(pb); /*
> > duration */
> >
> > +if ((version == 1 && st->duration == UINT64_MAX) ||
> > +(version != 1 && st->duration == UINT32_MAX)) {
> > +st->duration = 0;
> > +}
> > +
> >  lang = avio_rb16(pb); /* language */
> >  if (ff_mov_lang_to_iso639(lang, language))
> >  av_dict_set(&st->metadata, "language", language, 0);
> > --
> > 2.8.0.rc3.226.g39d4020
> >
> > ___
> > 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 mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] mov: only read e_old if there were any old streams

2016-10-31 Thread Sasi Inguva
patch looks good to me. Thanks for the fix.

On Mon, Oct 31, 2016 at 5:17 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:

> On 31.10.2016 19:20, Sasi Inguva wrote:
> > First of all, if nb_old == 0 i.e. there are no entries in AVIndex, then
> > there is no point in calling mov_fix_index function at all. So instead of
> > doing the above , you can directly check for st->nb_index_entries > 0 at
> > the top of mov_fix_index and return otherwise.
>
> OK, patch doing that is attached.
>
> > Also, I don't understand how nb_old==0 can cause heap overflow. If I read
> > the code correctly, if nb_old==0  find_prev_closest_keyframe_index ,
> should
> > return -1, which would make the function skip that edit list here
> >
> > if (index == -1) {
> >av_log(mov>->fc, AV_LOG_ERROR, "Missing key frame while
> reordering index according to edit list\n");
> >   continue;
> >}
>
> This checks is four lines below the heap buffer overflow, which is
> obviously too late.
>
> Best regards,
> Andreas
>
> ___
> 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: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 3:30 PM, Nicolas George wrote:
> Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
>> It's an scenario that could happen, like it or not. I'd rather not open the
>> doors for it.
> 
> "Opening doors" sounds like a different wording for the slippery slope
> fallacy.

Would you prefer "setting a precedent", then?

> 
>> "why this format and not this other" is coincidentally the argument that
>> will be used after this thing gets committed to justify adding others.
> 
> And the answer in this case is very easy: if the format is more useful
> than harmful, it goes in.

No. You insist this is about technical merits, and i already mentioned this
is not about that.

> 
>> If something is to create these files, it should be ffprobe itself and
>> not some unrelated muxer that's going to duplicate a lot of code.
> 
> I agree, that would be best.
> 
>> It might, hence why i suggested this being implemented as a standalone tool
>> and not that you should drop the whole thing.
> 
> Ok, thanks.
> 
>> You presented an scenario and use case to dump media streams into a text
>> file to easily alter them, analyze them and reconstruct them. You then
>> declared said dumps to be an actual format so they could qualify for an
>> avformat module that could digest them.
>>
>> There are no technical drawbacks for this demuxer.
> 
> Again, thanks. Therefore, let us discuss the rest:
> 
>>   This is a concept and
>> design issue. It's a very specific need so far a single person or two
>> that's being forced into the libraries because it's the easiest way to
>> implement it.
>>
>> This should be written as a standalone tool that uses lavf API to rebuild
>> the custom xml-like dump with the hex formatted packets as created by
>> ffprobe. Basically, write ffmpeg's own, more powerful and versatile
>> version of oggz-dump that uses lavf instead of libogg, meaning the dumps
>> can be created and rebuilt from and into any lavf supported format.
> 
> Ok. But compared with the demuxer approach, I see several drawbacks
> (some of which I have already posted):
> 
> - More code, therefore more maintenance burden.
> 
> - When used for debugging (this is the main purpose of this feature),
>   more code path, and therefore more room to trigger unrelated bugs that
>   will waste time.
> 
> - Subject to the limitations of the intermediate format.
> 
>   This one is especially important: imagine I am trying to investigate a
>   bug that happens with files with inconsistent timestamps. I can edit
>   the dump to make the timestamps inconsistent. And then... I try to
>   rebuild a file based on the dump, and lavf (used by the undump tool)
>   forbids me to mux inconsistent timestamps. Game over.
> 
>   The whole point of this demuxer is to be able to build slightly broken
>   input, because they are the cases that need debugging the most.
>   Unfortunately, if you convert to an actual format, some of the
>   brokenness will be fixed on the fly or flat out impossible.
> 
> - More steps for the person who is doing the debugging. Considering some
>   debugging sessions will require hundreds of rounds, the difference
>   will add up. Not just the generation step, but also the half hour lost
>   because the developer forgot to convert after a change in the text
>   file.
> 
> Now, your move:

This is not a competition, Nicolas, and this is not a technical merits
discussion. Nobody doubts a lavf module would be much more versatile and
powerful than a standalone tool.

> 
> * What benefits do you see for the separate tool approach?
> 
> * Can you negate any of these drawbacks?

I'll repeat what i said. This is not a technical discussion. This is a
design issue. For your very specific debugging needs and scenario, you
devised a way to dump media streams into a text file and declared said 
dumps a multimedia "format" so they may qualify for a demuxer.
Libavformat is not a dumping ground for code molded by a single person's
specific needs, and it is not a library meant to hold your or anyone's
debug tools.

At least three developers have stated being against the implementation
of such an arbitrary custom format, and as much as you want to cover your
ears, declare it's all "duplicate arguments" in order to downplay and
discard their opinions, and decree that no matter how many people are
against something from a design PoV the only thing that matters are
technical arguments limited to the realm of how to implement the solution
for the very specific scenario you're presenting, if more people are
against it than in favor then it should not go in, or a vote should be
called.

And why not just keep it in a separate repo for that matter? Why does it
need to be committed to the main repo?
You talk about maintenance, yet keeping it in a separate repository would
mean as much maintenance for you as it would in the main repository.

> 
> Regards,
> 
> 
> 
> ___
>