Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata

[wm4]

On Fri, 27 Jan 2017 08:26:26 +0100
Carl Eugen Hoyos  wrote:

> 2017-01-27 7:04 GMT+01:00 wm4 :
> > On Thu, 26 Jan 2017 18:06:39 +0100
> > Carl Eugen Hoyos  wrote:
> >  
> >> 2017-01-26 9:26 GMT+01:00 wm4 :  
> >> > On Thu, 26 Jan 2017 09:16:00 +0100
> >> > Carl Eugen Hoyos  wrote:
> >> >  
> >> >> 2017-01-26 9:07 GMT+01:00 wm4 :
> >> >>  
> >> >> >> >> > Any metadata you export can and will get copied to a new file 
> >> >> >> >> > when
> >> >> >> >> > remuxing, therefor exporting arbitrary info that isn't actual 
> >> >> >> >> > stream
> >> >> >> >> > metadata tags in metadata is problematic - it carries over to 
> >> >> >> >> > the
> >> >> >> >> > destination file, in which it would be entirely meaningless.  
> >> >> >> >>
> >> >> >> >> Sorry, I don't understand:
> >> >> >> >> Which application do you mean?  
> >> >> >> >
> >> >> >> > ffmpeg  
> >> >> >>
> >> >> >> Didn't I ping yesterday a patch that avoids this?
> >> >> >> And wasn't this the mail you answered?  
> >> >> >
> >> >> > Sorry, I couldn't find it just now. What was the subject line?  
> >> >>
> >> >> My mailer (gmail) shows it as the first mail of this thread, so
> >> >> it (hopefully) uses the same subject.  
> >> >
> >> > Sorry, I missed that. But this code is not called for remuxing,
> >> > or is it?  
> >>
> >> Afair, this behaviour (copying the vendor on remuxing) is
> >> consistent with the Apple documentation.  
> >
> > What if you remux to mkv?  
> 
> Afair, copying the vendor tag on remuxing is consistent with
> its documentation.

Well, the Apple docs don't have anything to do with mkv?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata

[Carl Eugen Hoyos]

2017-01-27 7:04 GMT+01:00 wm4 :
> On Thu, 26 Jan 2017 18:06:39 +0100
> Carl Eugen Hoyos  wrote:
>
>> 2017-01-26 9:26 GMT+01:00 wm4 :
>> > On Thu, 26 Jan 2017 09:16:00 +0100
>> > Carl Eugen Hoyos  wrote:
>> >
>> >> 2017-01-26 9:07 GMT+01:00 wm4 :
>> >>
>> >> >> >> > Any metadata you export can and will get copied to a new file when
>> >> >> >> > remuxing, therefor exporting arbitrary info that isn't actual 
>> >> >> >> > stream
>> >> >> >> > metadata tags in metadata is problematic - it carries over to the
>> >> >> >> > destination file, in which it would be entirely meaningless.
>> >> >> >>
>> >> >> >> Sorry, I don't understand:
>> >> >> >> Which application do you mean?
>> >> >> >
>> >> >> > ffmpeg
>> >> >>
>> >> >> Didn't I ping yesterday a patch that avoids this?
>> >> >> And wasn't this the mail you answered?
>> >> >
>> >> > Sorry, I couldn't find it just now. What was the subject line?
>> >>
>> >> My mailer (gmail) shows it as the first mail of this thread, so
>> >> it (hopefully) uses the same subject.
>> >
>> > Sorry, I missed that. But this code is not called for remuxing,
>> > or is it?
>>
>> Afair, this behaviour (copying the vendor on remuxing) is
>> consistent with the Apple documentation.
>
> What if you remux to mkv?

Afair, copying the vendor tag on remuxing is consistent with
its documentation.

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

Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata

[wm4]

On Thu, 26 Jan 2017 18:06:39 +0100
Carl Eugen Hoyos  wrote:

> 2017-01-26 9:26 GMT+01:00 wm4 :
> > On Thu, 26 Jan 2017 09:16:00 +0100
> > Carl Eugen Hoyos  wrote:
> >  
> >> 2017-01-26 9:07 GMT+01:00 wm4 :
> >>  
> >> >> >> > Any metadata you export can and will get copied to a new file when
> >> >> >> > remuxing, therefor exporting arbitrary info that isn't actual 
> >> >> >> > stream
> >> >> >> > metadata tags in metadata is problematic - it carries over to the
> >> >> >> > destination file, in which it would be entirely meaningless.  
> >> >> >>
> >> >> >> Sorry, I don't understand:
> >> >> >> Which application do you mean?  
> >> >> >
> >> >> > ffmpeg  
> >> >>
> >> >> Didn't I ping yesterday a patch that avoids this?
> >> >> And wasn't this the mail you answered?  
> >> >
> >> > Sorry, I couldn't find it just now. What was the subject line?  
> >>
> >> My mailer (gmail) shows it as the first mail of this thread, so
> >> it (hopefully) uses the same subject.  
> >
> > Sorry, I missed that. But this code is not called for remuxing,
> > or is it?  
> 
> Afair, this behaviour (copying the vendor on remuxing) is
> consistent with the Apple documentation.
> 

What if you remux to mkv?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[wm4]

On Fri, 27 Jan 2017 03:03:03 +0100
Michael Niedermayer  wrote:

> On Thu, Jan 26, 2017 at 02:29:21PM +0100, Paul Arzelier wrote:
> > Alright, attached is the last version (I hope!)
> > 
> > Paul
> > 
> > 
> > Le 26/01/2017 à 13:43, wm4 a écrit :  
> > >On Thu, 26 Jan 2017 13:32:08 +0100
> > >Paul Arzelier  wrote:
> > >  
> > >> From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
> > >>From: Polochon-street 
> > >>Date: Thu, 26 Jan 2017 13:25:22 +0100
> > >>Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis
> > >>  comments)
> > >>Originally-by: Ben Boeckel 
> > >>---
> > >>  libavformat/utils.c | 17 -
> > >>  1 file changed, 16 insertions(+), 1 deletion(-)
> > >>  
> > >Patch looks generally great to me now.
> > >  
> > >>diff --git a/libavformat/utils.c b/libavformat/utils.c
> > >>index d5dfca7dec..ce24244135 100644
> > >>--- a/libavformat/utils.c
> > >>+++ b/libavformat/utils.c
> > >>@@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const 
> > >>char *filename,
> > >>  AVFormatContext *s = *ps;
> > >>  int i, ret = 0;
> > >>  AVDictionary *tmp = NULL;
> > >>+AVDictionary *id3_meta = NULL;
> > >>  ID3v2ExtraMeta *id3v2_extra_meta = NULL;
> > >>  if (!s && !(s = avformat_alloc_context()))
> > >>@@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const 
> > >>char *filename,
> > >>  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
> > >>  if (s->pb)
> > >>-ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
> > >>+ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, 
> > >>_extra_meta);
> > >>  if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)
> > >>  if ((ret = s->iformat->read_header(s)) < 0)
> > >>  goto fail;
> > >>+if (!s->metadata) {
> > >>+av_dict_copy(>metadata, id3_meta, AV_DICT_DONT_OVERWRITE);
> > >>+av_dict_free(_meta);  
> > >Strictly speaking, this line is redundant, but it doesn't harm either.
> > >If you feel like it you could remove it, but it's not necessary.
> > >  
> > >>+}
> > >>+else if (id3_meta) {  
> > >If you make another patch, you could merge the } with the next line too
> > >(I'm not sure, but I think that's preferred coding-style wise).
> > >  
> > >>+int level = AV_LOG_WARNING;
> > >>+if (s->error_recognition & AV_EF_COMPLIANT)
> > >>+level = AV_LOG_ERROR;
> > >>+av_log(s, level, "Discarding ID3 tag because more suitable tags 
> > >>were found.\n");
> > >>+if (s->error_recognition & AV_EF_EXPLODE)
> > >>+return AVERROR_INVALIDDATA;
> > >>+}
> > >>+
> > >>  if (id3v2_extra_meta) {
> > >>  if (!strcmp(s->iformat->name, "mp3") || 
> > >> !strcmp(s->iformat->name, "aac") ||
> > >>  !strcmp(s->iformat->name, "tta")) {
> > >>@@ -627,6 +641,7 @@ int avformat_open_input(AVFormatContext **ps, const 
> > >>char *filename,
> > >>  fail:
> > >>  ff_id3v2_free_extra_meta(_extra_meta);
> > >>  av_dict_free();
> > >>+   av_dict_free(_meta);
> > >>  if (s->pb && !(s->flags & AVFMT_FLAG_CUSTOM_IO))
> > >>  avio_closep(>pb);
> > >>  avformat_free_context(s);  
> > >___
> > >ffmpeg-devel mailing list
> > >ffmpeg-devel@ffmpeg.org
> > >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel  
> > 
> > -- 
> > Paul ARZELIER
> > Élève ingénieur à l'École Centrale de Lille
> > 2014-2017
> >   
> 
> >  utils.c |   17 -
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 477d4f87058a098464e2c1dbfe7e7ff806d2c85f  
> > 0001-Ignore-ID3-tags-if-other-tags-are-present-e.g.-vorbi.patch
> > From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
> > From: Polochon-street 
> > Date: Thu, 26 Jan 2017 13:25:22 +0100
> > Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis 
> >  comments)
> > Originally-by: Ben Boeckel 
> > ---
> >  libavformat/utils.c | 17 -
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index d5dfca7dec..ce24244135 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const 
> > char *filename,
> >  AVFormatContext *s = *ps;
> >  int i, ret = 0;
> >  AVDictionary *tmp = NULL;
> > +AVDictionary *id3_meta = NULL;
> >  ID3v2ExtraMeta *id3v2_extra_meta = NULL;
> >  
> >  if (!s && !(s = avformat_alloc_context()))
> > @@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const 
> > char *filename,
> >  
> >  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
> >  if (s->pb)
> > -ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
> > +   

Re: [FFmpeg-devel] [PATCH] HTTP: optimize forward seek performance

[Joel Cunningham]

Attached is a git format-patch file



0001-HTTP-improve-performance-by-reducing-forward-seeks.patch
Description: Binary data

> On Jan 26, 2017, at 6:52 PM, Michael Niedermayer  wrote:
> 
> On Thu, Jan 26, 2017 at 10:44:33AM -0600, Joel Cunningham wrote:
>> Michael,
>> 
>> Thanks for the review and testing!  New patch included, see comments below
>> 
>>> 
>>> this segfaults with many fuzzed files
>>> backtrace looks like this:
>>> 
>>> #0  0x7fffb368 in ?? ()
>>> #1  0x005f9280 in avio_seek (s=0x7fffb278, offset=31, whence=0) 
>>> at libavformat/aviobuf.c:259
>>> 
>> 
>> I found I wasn’t setting s->short_seek_get to NULL in ffio_init_context() 
>> and was relying on the zeroing aspect of av_mallocz() in 
>> avio_alloc_context().  From a grep of the source, it looks like 
>> fifo_init_context() can be called from other functions without having zero’d 
>> the AVIOContext first.
>> 
>> Is the fuzz test exercising the AVIOContext in this manner?  I’d also like 
>> to reproduce this test if there are instructions
>> 
 
 diff --git a/libavformat/avio.c b/libavformat/avio.c
 index 3606eb0..ecf6801 100644
 --- a/libavformat/avio.c
 +++ b/libavformat/avio.c
 @@ -645,6 +645,13 @@ int ffurl_get_multi_file_handle(URLContext *h, int 
 **handles, int *numhandles)
return h->prot->url_get_multi_file_handle(h, handles, numhandles);
 }
 
 +int ffurl_get_short_seek(URLContext *h)
 +{
 +if (!h->prot->url_get_short_seek)
>>> 
 +return -1;
>>> 
>>> should be some AVERROR code
>> 
>> Fixed
>> 
 +return h->prot->url_get_short_seek(h);
 +}
 +
 int ffurl_shutdown(URLContext *h, int flags)
 {
if (!h->prot->url_shutdown)
 diff --git a/libavformat/avio.h b/libavformat/avio.h
 index b1ce1d1..0480981 100644
 --- a/libavformat/avio.h
 +++ b/libavformat/avio.h
 @@ -287,6 +287,12 @@ typedef struct AVIOContext {
int short_seek_threshold;
 
/**
 + * A callback that is used instead of short_seek_threshold.
 + * This is current internal only, do not use from outside.
 + */
 +int (*short_seek_get)(void *opaque);
 +
 +/**
 * ',' separated list of allowed protocols.
 */
const char *protocol_whitelist;
>>> 
>>> thats not ok to add this way, the docs say this:
>>> /**
>>> * Bytestream IO Context.
>>> * New fields can be added to the end with minor version bumps.
>>> * Removal, reordering and changes to existing fields require a major
>>> * version bump.
>>> * sizeof(AVIOContext) must not be used outside libav*.
>>> *
>>> * @note None of the function pointers in AVIOContext should be called
>>> *   directly, they should only be set by the client application
>>> *   when implementing custom I/O. Normally these are set to the
>>> *   function pointers specified in avio_alloc_context()
>>> */
>>> 
>> 
>> I moved short_seek_get to the end of AVIOContext.  I’m not sure if the minor 
>> version bump referenced in the comment requires any in-code changes.  Is 
>> this an acceptable magnitude of change for this feature?
>> 
>>> [...]
 --- a/libavformat/tcp.c
 +++ b/libavformat/tcp.c
 @@ -265,6 +265,26 @@ static int tcp_get_file_handle(URLContext *h)
return s->fd;
 }
 
 +static int tcp_get_window_size(URLContext *h)
 +{
 +TCPContext *s = h->priv_data;
 +int avail;
 +int avail_len = sizeof(avail);
 +
>>> 
 +#if HAVE_WINSOCK2_H
>>> 
>>> this formating is IIRC not allowed in C the # must be in the first
>>> column
>>> 
>>> 
 +/* SO_RCVBUF with winsock only reports the actual TCP window size when
 +auto-tuning has been disabled via setting SO_RCVBUF */
 +if (s->recv_buffer_size < 0) {
 +return AVERROR(ENOSYS);
 +}
 +#endif
 +
 +if (getsockopt(s->fd, SOL_SOCKET, SO_RCVBUF, , _len)) {
 +   return ff_neterrno();
 +}
>>> 
>>> the indention is inconsisntent
>> 
>> Both formatting issues have been corrected
>> 
>> From bac0f351afd14c56c56376d20557b7330bcea23e Mon Sep 17 00:00:00 2001
>> From: Joel Cunningham 
>> Date: Fri, 13 Jan 2017 10:52:25 -0600
>> Subject: [PATCH] HTTP: improve performance by reducing forward seeks
> 
> 
> please attach a proper git format-patch or use git send-email
> 
> applying this is not as smooth and easy as it should be
> 
> [...]
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> If you fake or manipulate statistics in a paper in physics you will never
> get a job again.
> If you fake or manipulate statistics in a paper in medicin you will get
> a job for life at the pharma industry.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] GSoC 2017

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 03:34:24PM -0200, Pedro Arthur wrote:
> Hi all,
> I can be a backup mentor if needed.

we need more (backup) mentors, so yes, i think its needed

also if you want to be mentor for a swscale related project i would
support that idea


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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus


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

Re: [FFmpeg-devel] [PATCH 9/9] boadec: prevent overflow during block alignment calculation

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 02:14:09AM +0100, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/boadec.c | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)

LGTM

thx

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

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.


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

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 02:29:21PM +0100, Paul Arzelier wrote:
> Alright, attached is the last version (I hope!)
> 
> Paul
> 
> 
> Le 26/01/2017 à 13:43, wm4 a écrit :
> >On Thu, 26 Jan 2017 13:32:08 +0100
> >Paul Arzelier  wrote:
> >
> >> From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
> >>From: Polochon-street 
> >>Date: Thu, 26 Jan 2017 13:25:22 +0100
> >>Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis
> >>  comments)
> >>Originally-by: Ben Boeckel 
> >>---
> >>  libavformat/utils.c | 17 -
> >>  1 file changed, 16 insertions(+), 1 deletion(-)
> >>
> >Patch looks generally great to me now.
> >
> >>diff --git a/libavformat/utils.c b/libavformat/utils.c
> >>index d5dfca7dec..ce24244135 100644
> >>--- a/libavformat/utils.c
> >>+++ b/libavformat/utils.c
> >>@@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const 
> >>char *filename,
> >>  AVFormatContext *s = *ps;
> >>  int i, ret = 0;
> >>  AVDictionary *tmp = NULL;
> >>+AVDictionary *id3_meta = NULL;
> >>  ID3v2ExtraMeta *id3v2_extra_meta = NULL;
> >>  if (!s && !(s = avformat_alloc_context()))
> >>@@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const 
> >>char *filename,
> >>  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
> >>  if (s->pb)
> >>-ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
> >>+ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, 
> >>_extra_meta);
> >>  if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)
> >>  if ((ret = s->iformat->read_header(s)) < 0)
> >>  goto fail;
> >>+if (!s->metadata) {
> >>+av_dict_copy(>metadata, id3_meta, AV_DICT_DONT_OVERWRITE);
> >>+av_dict_free(_meta);
> >Strictly speaking, this line is redundant, but it doesn't harm either.
> >If you feel like it you could remove it, but it's not necessary.
> >
> >>+}
> >>+else if (id3_meta) {
> >If you make another patch, you could merge the } with the next line too
> >(I'm not sure, but I think that's preferred coding-style wise).
> >
> >>+int level = AV_LOG_WARNING;
> >>+if (s->error_recognition & AV_EF_COMPLIANT)
> >>+level = AV_LOG_ERROR;
> >>+av_log(s, level, "Discarding ID3 tag because more suitable tags 
> >>were found.\n");
> >>+if (s->error_recognition & AV_EF_EXPLODE)
> >>+return AVERROR_INVALIDDATA;
> >>+}
> >>+
> >>  if (id3v2_extra_meta) {
> >>  if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, 
> >> "aac") ||
> >>  !strcmp(s->iformat->name, "tta")) {
> >>@@ -627,6 +641,7 @@ int avformat_open_input(AVFormatContext **ps, const 
> >>char *filename,
> >>  fail:
> >>  ff_id3v2_free_extra_meta(_extra_meta);
> >>  av_dict_free();
> >>+   av_dict_free(_meta);
> >>  if (s->pb && !(s->flags & AVFMT_FLAG_CUSTOM_IO))
> >>  avio_closep(>pb);
> >>  avformat_free_context(s);
> >___
> >ffmpeg-devel mailing list
> >ffmpeg-devel@ffmpeg.org
> >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> -- 
> Paul ARZELIER
> Élève ingénieur à l'École Centrale de Lille
> 2014-2017
> 

>  utils.c |   17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 477d4f87058a098464e2c1dbfe7e7ff806d2c85f  
> 0001-Ignore-ID3-tags-if-other-tags-are-present-e.g.-vorbi.patch
> From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
> From: Polochon-street 
> Date: Thu, 26 Jan 2017 13:25:22 +0100
> Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis 
>  comments)
> Originally-by: Ben Boeckel 
> ---
>  libavformat/utils.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index d5dfca7dec..ce24244135 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
> *filename,
>  AVFormatContext *s = *ps;
>  int i, ret = 0;
>  AVDictionary *tmp = NULL;
> +AVDictionary *id3_meta = NULL;
>  ID3v2ExtraMeta *id3v2_extra_meta = NULL;
>  
>  if (!s && !(s = avformat_alloc_context()))
> @@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const 
> char *filename,
>  
>  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
>  if (s->pb)
> -ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
> +ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, 
> _extra_meta);
>  
>  if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)
>  if ((ret = s->iformat->read_header(s)) < 0)
>  goto fail;
>  
> +if (!s->metadata) {
> +s->metadata = id3_meta;
> +id3_meta = NULL;
> +} else if (id3_meta) 

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Marton Balint]

On Thu, 26 Jan 2017, Michael Niedermayer wrote:


On Thu, Jan 26, 2017 at 03:52:04AM +0100, Marton Balint wrote:


On Thu, 26 Jan 2017, Michael Niedermayer wrote:


On Thu, Jan 26, 2017 at 02:58:07AM +0100, Andreas Cadhalpun wrote:

On 26.01.2017 02:29, Ronald S. Bultje wrote:

On Wed, Jan 25, 2017 at 8:12 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:


Signed-off-by: Andreas Cadhalpun 
---
libavformat/nistspheredec.c | 11 +++
1 file changed, 11 insertions(+)

diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c
index 782d1dfbfb..3386497682 100644
--- a/libavformat/nistspheredec.c
+++ b/libavformat/nistspheredec.c
@@ -21,6 +21,7 @@

#include "libavutil/avstring.h"
#include "libavutil/intreadwrite.h"
+#include "libavcodec/internal.h"
#include "avformat.h"
#include "internal.h"
#include "pcm.h"
@@ -90,6 +91,11 @@ static int nist_read_header(AVFormatContext *s)
return 0;
} else if (!memcmp(buffer, "channel_count", 13)) {
sscanf(buffer, "%*s %*s %"SCNd32, >codecpar->channels);
+if (st->codecpar->channels > FF_SANE_NB_CHANNELS) {
+av_log(s, AV_LOG_ERROR, "Too many channels %d > %d\n",
+   st->codecpar->channels, FF_SANE_NB_CHANNELS);
+return AVERROR(ENOSYS);
+}



I've said this before, but again - please don't add useless log messages.


I disagree that these log messages are useless and I'm not the only one [1].


+1

Log messages make debuging the code easier, as a developer i like to
know why something fails having a hard failure but no clear and easy
findable error message is bad



I tend to agree with Ronald here, log messages which are practically
impossible to trigger with real-world files have little benefit,
also I don't think it is ffmpeg's job to thoroughly explain every
different kind of error.


would you want a log message be removed if the
people working on the code want the error message to be there ?
You seem to just say that you see little benefit in it.



Yeah, because as far as I understand the reason behind this, it is 
only there to let the fuzzer people know why a fuzzed file fails. If we 
don't draw the line somewhere, ffmpeg will be an analyzer and not a 
decoder.


I see 3 problems (wm4 explicitly named them, but I also had them in mind)
- Little benefit, yet
- Makes the code less clean, more cluttered
- Increases binary size

The ideas I proposed (use macros, use common / factorized checks for 
common validatons and errors) might be a good compromise IMHO. Fuzzing 
thereforce can be done with "debug" builds.


Anyway, I am not blocking the patch, just expressing what I would prefer 
in the long run.


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

Re: [FFmpeg-devel] GSoC 2017

[Mayank Agarwal]

Hi,

I am interested in joining one of the projects.

Regards
Mayank


On Thu, Jan 26, 2017 at 11:04 PM, Pedro Arthur  wrote:

> Hi all,
> I can be a backup mentor if needed.
>
> 2017-01-25 10:15 GMT-02:00 Thilo Borgmann :
>
> > Am 25.01.17 um 06:14 schrieb Umair Khan:
> > > On Wed, Jan 25, 2017 at 7:45 AM, Michael Niedermayer  >
> > wrote:
> > >>
> > >> On Mon, Jan 23, 2017 at 03:39:09PM +0100, Michael Niedermayer wrote:
> > >>> Hi all
> > >>>
> > >>> GSoC 2017 mentor org registration has opened a few days ago
> > >>>
> > >>> Our current ideas page lists just a single mentor and single
> sugestion
> > >>> http://trac.ffmpeg.org/wiki/SponsoringPrograms/GSoC/2017
> > >>>
> > >>> About the Suggested project
> > >>> It is a hard design task which few students would be
> > >>> capable to do. I dont know if someone already knows a student who is
> > >>> qualified for this and who would be available in FFmpeg GSoC but if
> not
> > >>> I think finding a qualified student will be hard or require some luck
> > >>> and success with a less than qualified student unlikely.
> > >>> At least thats what my gut feeling says based on past years
> > >>>
> > >>> About the number of ideas
> > >>> one is not enough, and i think its a waste of time if we apply with
> > >>> such short list, i doubt we would be accepted with one idea and one
> > >>> mentor
> > >>>
> > >>> About the number of mentors
> > >>> one is not enough
> > >>
> > >> ok so our page now contains
> > >> 4 project ideas and 3 mentors
> > >> big thanks to the people volunteering and adding them
> > >>
> > >> Can we get a few more ?
> > >> Ideal would be numbers similar to previous years
> > >> We wont have a student for each idea we list most likely.
> > >> If we list just 4 we might have just 1 or 2 students, having a
> > >> wider range of ideas to select from would increase the number of
> > >> contributions and number of potential new developers joining.
> > >
> > > One idea would be to finish rest of the ALS codec tasks.
> > > What's remaining is :-
> >
> > > 1. Implement RLS LMS mode in the decoder.
> >
> > See 3.
> >
> >
> > > 2. Get the encoder merged into the main repository.
> >
> > This was part of last years (yours) project... I admit that this has not
> > yet happened is due to my lack of time for working on it with you (which
> > will most likely change soon :) )
> > However I would not like to have this iterated but being finished by the
> > start of the next GSoC round (which basically means by yesterday...).
> >
> >
> > > 3. Encoder featureset still lacks support for floating point sample
> > > data encoding.
> >
> > 1. & 3. could be part of a "complete feature set for ALS"-project. Both
> > are too little for separated projects and too much for a qualification
> > task. But I like the idea of a comprehensive feature set project. You
> came
> > up with this idea first so if you're up to mentor it, go ahead and add it
> > to the wiki. Otherwise I'll do it during the weekend.
> >
> > -Thilo
> >
> > ___
> > 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] lavc/mjpegdec: hint next marker position in ff_mjpeg_find_marker

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 05:47:51PM +0100, Matthieu Bouron wrote:
> Speeds up next marker search when a SOS marker is found.
> 
> ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
> sample_2992x4000.jpg
> ---
>  libavcodec/mjpegdec.c | 31 +--
>  libavcodec/mjpegdec.h |  2 +-
>  libavcodec/mxpegdec.c |  5 +++--
>  3 files changed, 29 insertions(+), 9 deletions(-)

this breaks decoding multiple jpeg files
for example tickets//856/nint.jpg

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

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato


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

[FFmpeg-devel] [PATCH] doc/muxers: remove confusing example for segment muxer option clocktime_wrap_duration

[Marton Balint]

Detecting a leap second depends on a lot of things, segment time, segment
offset, system leap second implementation, the removed part is a huge
simplification which can be misleading, so it is best to remove it.

Signed-off-by: Marton Balint 
---
 doc/muxers.texi | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 26a8f2d..2438ab2 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1408,9 +1408,6 @@ within the specified duration after the segmenting clock 
time. This way you
 can make the segmenter more resilient to backward local time jumps, such as
 leap seconds or transition to standard time from daylight savings time.
 
-Assuming that the delay between the packets of your source is less than 0.5
-second you can detect a leap second by specifying 0.5 as the duration.
-
 Default is the maximum possible duration which means starting a new segment
 regardless of the elapsed time since the last clock time.
 
-- 
2.10.2

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

Re: [FFmpeg-devel] [PATCH] HTTP: optimize forward seek performance

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 10:44:33AM -0600, Joel Cunningham wrote:
> Michael,
> 
> Thanks for the review and testing!  New patch included, see comments below
> 
> > 
> > this segfaults with many fuzzed files
> > backtrace looks like this:
> > 
> > #0  0x7fffb368 in ?? ()
> > #1  0x005f9280 in avio_seek (s=0x7fffb278, offset=31, whence=0) 
> > at libavformat/aviobuf.c:259
> > 
> 
> I found I wasn’t setting s->short_seek_get to NULL in ffio_init_context() and 
> was relying on the zeroing aspect of av_mallocz() in avio_alloc_context().  
> From a grep of the source, it looks like fifo_init_context() can be called 
> from other functions without having zero’d the AVIOContext first.
> 
> Is the fuzz test exercising the AVIOContext in this manner?  I’d also like to 
> reproduce this test if there are instructions
> 
> >> 
> >> diff --git a/libavformat/avio.c b/libavformat/avio.c
> >> index 3606eb0..ecf6801 100644
> >> --- a/libavformat/avio.c
> >> +++ b/libavformat/avio.c
> >> @@ -645,6 +645,13 @@ int ffurl_get_multi_file_handle(URLContext *h, int 
> >> **handles, int *numhandles)
> >> return h->prot->url_get_multi_file_handle(h, handles, numhandles);
> >> }
> >> 
> >> +int ffurl_get_short_seek(URLContext *h)
> >> +{
> >> +if (!h->prot->url_get_short_seek)
> > 
> >> +return -1;
> > 
> > should be some AVERROR code
> 
> Fixed
> 
> >> +return h->prot->url_get_short_seek(h);
> >> +}
> >> +
> >> int ffurl_shutdown(URLContext *h, int flags)
> >> {
> >> if (!h->prot->url_shutdown)
> >> diff --git a/libavformat/avio.h b/libavformat/avio.h
> >> index b1ce1d1..0480981 100644
> >> --- a/libavformat/avio.h
> >> +++ b/libavformat/avio.h
> >> @@ -287,6 +287,12 @@ typedef struct AVIOContext {
> >> int short_seek_threshold;
> >> 
> >> /**
> >> + * A callback that is used instead of short_seek_threshold.
> >> + * This is current internal only, do not use from outside.
> >> + */
> >> +int (*short_seek_get)(void *opaque);
> >> +
> >> +/**
> >>  * ',' separated list of allowed protocols.
> >>  */
> >> const char *protocol_whitelist;
> > 
> > thats not ok to add this way, the docs say this:
> > /**
> > * Bytestream IO Context.
> > * New fields can be added to the end with minor version bumps.
> > * Removal, reordering and changes to existing fields require a major
> > * version bump.
> > * sizeof(AVIOContext) must not be used outside libav*.
> > *
> > * @note None of the function pointers in AVIOContext should be called
> > *   directly, they should only be set by the client application
> > *   when implementing custom I/O. Normally these are set to the
> > *   function pointers specified in avio_alloc_context()
> > */
> > 
> 
> I moved short_seek_get to the end of AVIOContext.  I’m not sure if the minor 
> version bump referenced in the comment requires any in-code changes.  Is this 
> an acceptable magnitude of change for this feature?
> 
> > [...]
> >> --- a/libavformat/tcp.c
> >> +++ b/libavformat/tcp.c
> >> @@ -265,6 +265,26 @@ static int tcp_get_file_handle(URLContext *h)
> >> return s->fd;
> >> }
> >> 
> >> +static int tcp_get_window_size(URLContext *h)
> >> +{
> >> +TCPContext *s = h->priv_data;
> >> +int avail;
> >> +int avail_len = sizeof(avail);
> >> +
> > 
> >> +#if HAVE_WINSOCK2_H
> > 
> > this formating is IIRC not allowed in C the # must be in the first
> > column
> > 
> > 
> >> +/* SO_RCVBUF with winsock only reports the actual TCP window size when
> >> +auto-tuning has been disabled via setting SO_RCVBUF */
> >> +if (s->recv_buffer_size < 0) {
> >> +return AVERROR(ENOSYS);
> >> +}
> >> +#endif
> >> +
> >> +if (getsockopt(s->fd, SOL_SOCKET, SO_RCVBUF, , _len)) {
> >> +   return ff_neterrno();
> >> +}
> > 
> > the indention is inconsisntent
> 
> Both formatting issues have been corrected
> 
> From bac0f351afd14c56c56376d20557b7330bcea23e Mon Sep 17 00:00:00 2001
> From: Joel Cunningham 
> Date: Fri, 13 Jan 2017 10:52:25 -0600
> Subject: [PATCH] HTTP: improve performance by reducing forward seeks


please attach a proper git format-patch or use git send-email

applying this is not as smooth and easy as it should be

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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


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

[FFmpeg-devel] [PATCH] avformat: parse iTunes gapless information

[Christopher Snowhill]

Signed-off-by: Christopher Snowhill 
---
 libavformat/mp3dec.c | 77 +++-
 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 099ca57..b895e37 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -295,6 +295,66 @@ static void mp3_parse_vbri_tag(AVFormatContext *s, 
AVStream *st, int64_t base)
 }
 }
 
+static void mp3_parse_itunes_tag(AVFormatContext *s, AVStream *st, 
MPADecodeHeader *c, int64_t base, int vbrtag_size, unsigned int *size, uint64_t 
*duration)
+{
+uint32_t v;
+AVDictionaryEntry *de;
+MP3DecContext *mp3 = s->priv_data;
+size_t length;
+uint32_t zero, start_pad, end_pad;
+uint64_t last_eight_frames_offset;
+int64_t temp_duration, file_size;
+int i;
+
+if (!s->metadata || !(de = av_dict_get(s->metadata, "iTunSMPB", NULL, 0)))
+return;
+
+length = strlen(de->value);
+
+/* Minimum length is one digit per field plus the whitespace, maximum 
length should depend on field type
+ * There are four fields we need in the first six, the rest are currently 
zero padding */
+if (length < (12 + 11) || length > (10 * 8 + 2 * 16 + 11))
+return;
+
+file_size = avio_size(s->pb);
+if (file_size < 0)
+file_size = 0;
+
+if (sscanf(de->value, "%"PRIx32" %"PRIx32" %"PRIx32" %"PRIx64" %"PRIx32" 
%"PRIx64, , _pad, _pad, _duration, , 
_eight_frames_offset) < 6 ||
+temp_duration < 0 ||
+start_pad > (576 * 2 * 32) ||
+end_pad > (576 * 2 * 64) ||
+(file_size && (last_eight_frames_offset >= (file_size - base - 
vbrtag_size {
+*duration = 0;
+return;
+}
+
+*duration = temp_duration;
+
+mp3->start_pad = start_pad;
+mp3->end_pad = end_pad;
+if (end_pad >= 528 + 1)
+mp3->end_pad = end_pad - (528 + 1);
+st->start_skip_samples = mp3->start_pad + 528 + 1;
+av_log(s, AV_LOG_DEBUG, "pad %d %d\n", mp3->start_pad, mp3->end_pad);
+if (!s->pb->seekable)
+return;
+
+*size = (unsigned int) last_eight_frames_offset;
+if (avio_seek(s->pb, base + vbrtag_size + last_eight_frames_offset, 
SEEK_SET) < 0)
+return;
+
+for (i = 0; i < 8; i++) {
+v = avio_rb32(s->pb);
+if (ff_mpa_check_header(v) < 0)
+return;
+if (avpriv_mpegaudio_decode_header(c, v) != 0)
+break;
+*size += c->frame_size;
+avio_skip(s->pb, c->frame_size - 4);
+}
+}
+
 /**
  * Try to find Xing/Info/VBRI tags and compute duration from info therein
  */
@@ -303,8 +363,10 @@ static int mp3_parse_vbr_tags(AVFormatContext *s, AVStream 
*st, int64_t base)
 uint32_t v, spf;
 MPADecodeHeader c;
 int vbrtag_size = 0;
+unsigned int size = 0;
 MP3DecContext *mp3 = s->priv_data;
 int ret;
+uint64_t duration = 0;
 
 ffio_init_checksum(s->pb, ff_crcA001_update, 0);
 
@@ -327,16 +389,29 @@ static int mp3_parse_vbr_tags(AVFormatContext *s, 
AVStream *st, int64_t base)
 mp3_parse_vbri_tag(s, st, base);
 
 if (!mp3->frames && !mp3->header_filesize)
+vbrtag_size = 0;
+
+mp3_parse_itunes_tag(s, st, , base, vbrtag_size, , );
+
+if (!mp3->frames && !size && !duration)
 return -1;
 
 /* Skip the vbr tag frame */
 avio_seek(s->pb, base + vbrtag_size, SEEK_SET);
 
-if (mp3->frames)
+if (duration)
+st->duration = av_rescale_q(duration, (AVRational){1, c.sample_rate}, 
st->time_base);
+else if (mp3->frames)
 st->duration = av_rescale_q(mp3->frames, (AVRational){spf, 
c.sample_rate},
 st->time_base);
 if (mp3->header_filesize && mp3->frames && !mp3->is_cbr)
 st->codecpar->bit_rate = av_rescale(mp3->header_filesize, 8 * 
c.sample_rate, mp3->frames * (int64_t)spf);
+if (size) {
+if (duration)
+st->codecpar->bit_rate = av_rescale(size, 8 * c.sample_rate, 
duration);
+else if (mp3->frames)
+st->codecpar->bit_rate = av_rescale(size, 8 * c.sample_rate, 
mp3->frames * (int64_t)spf);
+}
 
 return 0;
 }
-- 
2.10.1.windows.1

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

[FFmpeg-devel] [FFmpeg-devel, PATCHv5] avformat: parse iTunes gapless information

[Christopher Snowhill]

Let's try this again. I can't believe I missed all those things you
mentioned. It's almost as if I didn't actually proofread my patch
before sending it.

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

Re: [FFmpeg-devel] [PATCH 2/9] electronicarts: prevent overflow during block alignment calculation

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 02:11:31AM +0100, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/electronicarts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

thx

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

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


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

[FFmpeg-devel] [PATCH] lavf/matroskaenc.c: Free dyn bufs in mkv_free. Fixes memory leaks when muxing fails.

[Sasi Inguva]

Signed-off-by: Sasi Inguva 
---
 libavformat/matroskaenc.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index f731b678b9..88f6c647b9 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -393,6 +393,23 @@ static void put_xiph_size(AVIOContext *pb, int size)
  * Free the members allocated in the mux context.
  */
 static void mkv_free(MatroskaMuxContext *mkv) {
+uint8_t* buf;
+if (mkv->dyn_bc) {
+avio_close_dyn_buf(mkv->dyn_bc, );
+av_free(buf);
+}
+if (mkv->info_bc) {
+avio_close_dyn_buf(mkv->info_bc, );
+av_free(buf);
+}
+if (mkv->tracks_bc) {
+avio_close_dyn_buf(mkv->tracks_bc, );
+av_free(buf);
+}
+if (mkv->tags_bc) {
+avio_close_dyn_buf(mkv->tags_bc, );
+av_free(buf);
+}
 if (mkv->main_seekhead) {
 av_freep(>main_seekhead->entries);
 av_freep(>main_seekhead);
-- 
2.11.0.483.g087da7b7c-goog

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

Re: [FFmpeg-devel] [PATCH] avformat/Makefile: fix compilation of testprogs when networking is disabled

[Michael Niedermayer]

On Wed, Jan 25, 2017 at 03:05:55PM +0100, Tobias Rapp wrote:
> Signed-off-by: Tobias Rapp 
> ---
>  libavformat/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

probably ok

thx

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

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.


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

Re: [FFmpeg-devel] [PATCH] tcp: set socket buffer sizes before listen/connect/accept

[Michael Niedermayer]

On Wed, Jan 25, 2017 at 09:06:36AM -0600, Joel Cunningham wrote:
> Ping!  Anyone have a chance to look at this issue/patch?
> 
> Thanks,

patch applied

thx

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope


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

Re: [FFmpeg-devel] [PATCH] fate: add SCC test

[Michael Niedermayer]

On Wed, Jan 25, 2017 at 10:30:06PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  tests/fate/subtitles.mak |   3 +
>  tests/ref/fate/sub-scc   | 519 
> +++
>  2 files changed, 522 insertions(+)
>  create mode 100644 tests/ref/fate/sub-scc

Tested on linux32/64 x86, mingw32/64, mips/arm qemu linux

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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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

Re: [FFmpeg-devel] GSoC 2017

[Pedro Arthur]

Hi all,
I can be a backup mentor if needed.

2017-01-25 10:15 GMT-02:00 Thilo Borgmann :

> Am 25.01.17 um 06:14 schrieb Umair Khan:
> > On Wed, Jan 25, 2017 at 7:45 AM, Michael Niedermayer 
> wrote:
> >>
> >> On Mon, Jan 23, 2017 at 03:39:09PM +0100, Michael Niedermayer wrote:
> >>> Hi all
> >>>
> >>> GSoC 2017 mentor org registration has opened a few days ago
> >>>
> >>> Our current ideas page lists just a single mentor and single sugestion
> >>> http://trac.ffmpeg.org/wiki/SponsoringPrograms/GSoC/2017
> >>>
> >>> About the Suggested project
> >>> It is a hard design task which few students would be
> >>> capable to do. I dont know if someone already knows a student who is
> >>> qualified for this and who would be available in FFmpeg GSoC but if not
> >>> I think finding a qualified student will be hard or require some luck
> >>> and success with a less than qualified student unlikely.
> >>> At least thats what my gut feeling says based on past years
> >>>
> >>> About the number of ideas
> >>> one is not enough, and i think its a waste of time if we apply with
> >>> such short list, i doubt we would be accepted with one idea and one
> >>> mentor
> >>>
> >>> About the number of mentors
> >>> one is not enough
> >>
> >> ok so our page now contains
> >> 4 project ideas and 3 mentors
> >> big thanks to the people volunteering and adding them
> >>
> >> Can we get a few more ?
> >> Ideal would be numbers similar to previous years
> >> We wont have a student for each idea we list most likely.
> >> If we list just 4 we might have just 1 or 2 students, having a
> >> wider range of ideas to select from would increase the number of
> >> contributions and number of potential new developers joining.
> >
> > One idea would be to finish rest of the ALS codec tasks.
> > What's remaining is :-
>
> > 1. Implement RLS LMS mode in the decoder.
>
> See 3.
>
>
> > 2. Get the encoder merged into the main repository.
>
> This was part of last years (yours) project... I admit that this has not
> yet happened is due to my lack of time for working on it with you (which
> will most likely change soon :) )
> However I would not like to have this iterated but being finished by the
> start of the next GSoC round (which basically means by yesterday...).
>
>
> > 3. Encoder featureset still lacks support for floating point sample
> > data encoding.
>
> 1. & 3. could be part of a "complete feature set for ALS"-project. Both
> are too little for separated projects and too much for a qualification
> task. But I like the idea of a comprehensive feature set project. You came
> up with this idea first so if you're up to mentor it, go ahead and add it
> to the wiki. Otherwise I'll do it during the weekend.
>
> -Thilo
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata

[Carl Eugen Hoyos]

2017-01-26 9:26 GMT+01:00 wm4 :
> On Thu, 26 Jan 2017 09:16:00 +0100
> Carl Eugen Hoyos  wrote:
>
>> 2017-01-26 9:07 GMT+01:00 wm4 :
>>
>> >> >> > Any metadata you export can and will get copied to a new file when
>> >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream
>> >> >> > metadata tags in metadata is problematic - it carries over to the
>> >> >> > destination file, in which it would be entirely meaningless.
>> >> >>
>> >> >> Sorry, I don't understand:
>> >> >> Which application do you mean?
>> >> >
>> >> > ffmpeg
>> >>
>> >> Didn't I ping yesterday a patch that avoids this?
>> >> And wasn't this the mail you answered?
>> >
>> > Sorry, I couldn't find it just now. What was the subject line?
>>
>> My mailer (gmail) shows it as the first mail of this thread, so
>> it (hopefully) uses the same subject.
>
> Sorry, I missed that. But this code is not called for remuxing,
> or is it?

Afair, this behaviour (copying the vendor on remuxing) is
consistent with the Apple documentation.

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

[FFmpeg-devel] [PATCH] lavc/mjpegdec: hint next marker position in ff_mjpeg_find_marker

[Matthieu Bouron]

Speeds up next marker search when a SOS marker is found.

ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
sample_2992x4000.jpg
---
 libavcodec/mjpegdec.c | 31 +--
 libavcodec/mjpegdec.h |  2 +-
 libavcodec/mxpegdec.c |  5 +++--
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 7d17e3dfcb..29f40cb4e6 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -1921,13 +1921,23 @@ static int mjpeg_decode_com(MJpegDecodeContext *s)
 
 /* return the 8 bit start code value and update the search
state. Return -1 if no start code found */
-static int find_marker(const uint8_t **pbuf_ptr, const uint8_t *buf_end)
+static int find_marker(const uint8_t **pbuf_ptr, const uint8_t *buf_hint, 
const uint8_t *buf_end)
 {
 const uint8_t *buf_ptr;
 unsigned int v, v2;
 int val;
 int skipped = 0;
 
+if (buf_hint && (buf_end - buf_hint > 1)) {
+v = *buf_hint++;
+v2 = *buf_hint;
+if ((v == 0xff) && (v2 >= 0xc0) && (v2 <= 0xfe) && buf_hint < buf_end) 
{
+buf_ptr = buf_hint++;
+val = v2;
+goto found;
+}
+}
+
 buf_ptr = *pbuf_ptr;
 while (buf_end - buf_ptr > 1) {
 v  = *buf_ptr++;
@@ -1947,12 +1957,15 @@ found:
 }
 
 int ff_mjpeg_find_marker(MJpegDecodeContext *s,
- const uint8_t **buf_ptr, const uint8_t *buf_end,
+ const uint8_t **buf_ptr,
+ const uint8_t **buf_hint,
+ const uint8_t *buf_end,
  const uint8_t **unescaped_buf_ptr,
  int *unescaped_buf_size)
 {
 int start_code;
-start_code = find_marker(buf_ptr, buf_end);
+start_code = find_marker(buf_ptr, *buf_hint, buf_end);
+*buf_hint = NULL;
 
 av_fast_padded_malloc(>buffer, >buffer_size, buf_end - *buf_ptr);
 if (!s->buffer)
@@ -1963,6 +1976,7 @@ int ff_mjpeg_find_marker(MJpegDecodeContext *s,
 const uint8_t *src = *buf_ptr;
 const uint8_t *ptr = src;
 uint8_t *dst = s->buffer;
+const uint8_t *next_marker = NULL;
 
 #define copy_data_segment(skip) do {   \
 ptrdiff_t length = (ptr - src) - (skip);  \
@@ -1999,8 +2013,10 @@ int ff_mjpeg_find_marker(MJpegDecodeContext *s,
 
 if (x < 0xd0 || x > 0xd7) {
 copy_data_segment(1);
-if (x)
+if (x) {
+next_marker = ptr - 2;
 break;
+}
 }
 }
 }
@@ -2009,6 +2025,8 @@ int ff_mjpeg_find_marker(MJpegDecodeContext *s,
 }
 #undef copy_data_segment
 
+if (next_marker)
+*buf_hint = next_marker;
 *unescaped_buf_ptr  = s->buffer;
 *unescaped_buf_size = dst - s->buffer;
 memset(s->buffer + *unescaped_buf_size, 0,
@@ -2073,7 +2091,7 @@ int ff_mjpeg_decode_frame(AVCodecContext *avctx, void 
*data, int *got_frame,
 const uint8_t *buf = avpkt->data;
 int buf_size   = avpkt->size;
 MJpegDecodeContext *s = avctx->priv_data;
-const uint8_t *buf_end, *buf_ptr;
+const uint8_t *buf_end, *buf_hint, *buf_ptr;
 const uint8_t *unescaped_buf_ptr;
 int hshift, vshift;
 int unescaped_buf_size;
@@ -2087,10 +2105,11 @@ int ff_mjpeg_decode_frame(AVCodecContext *avctx, void 
*data, int *got_frame,
 s->adobe_transform = -1;
 
 buf_ptr = buf;
+buf_hint = NULL;
 buf_end = buf + buf_size;
 while (buf_ptr < buf_end) {
 /* find start next marker */
-start_code = ff_mjpeg_find_marker(s, _ptr, buf_end,
+start_code = ff_mjpeg_find_marker(s, _ptr, _hint, buf_end,
   _buf_ptr,
   _buf_size);
 /* EOF */
diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
index fb811294a1..c9d289187b 100644
--- a/libavcodec/mjpegdec.h
+++ b/libavcodec/mjpegdec.h
@@ -144,7 +144,7 @@ int ff_mjpeg_decode_sos(MJpegDecodeContext *s,
 const uint8_t *mb_bitmask,int mb_bitmask_size,
 const AVFrame *reference);
 int ff_mjpeg_find_marker(MJpegDecodeContext *s,
- const uint8_t **buf_ptr, const uint8_t *buf_end,
+ const uint8_t **buf_ptr, const uint8_t **buf_hint, 
const uint8_t *buf_end,
  const uint8_t **unescaped_buf_ptr, int 
*unescaped_buf_size);
 
 #endif /* AVCODEC_MJPEGDEC_H */
diff --git a/libavcodec/mxpegdec.c b/libavcodec/mxpegdec.c
index 2e3ebe6e70..28a479a3b5 100644
--- a/libavcodec/mxpegdec.c
+++ b/libavcodec/mxpegdec.c
@@ -189,18 +189,19 @@ static int mxpeg_decode_frame(AVCodecContext *avctx,
 int buf_size = avpkt->size;
 MXpegDecodeContext *s = avctx->priv_data;
 

Re: [FFmpeg-devel] [PATCH] HTTP: optimize forward seek performance

[Joel Cunningham]

Michael,

Thanks for the review and testing!  New patch included, see comments below

> 
> this segfaults with many fuzzed files
> backtrace looks like this:
> 
> #0  0x7fffb368 in ?? ()
> #1  0x005f9280 in avio_seek (s=0x7fffb278, offset=31, whence=0) 
> at libavformat/aviobuf.c:259
> 

I found I wasn’t setting s->short_seek_get to NULL in ffio_init_context() and 
was relying on the zeroing aspect of av_mallocz() in avio_alloc_context().  
From a grep of the source, it looks like fifo_init_context() can be called from 
other functions without having zero’d the AVIOContext first.

Is the fuzz test exercising the AVIOContext in this manner?  I’d also like to 
reproduce this test if there are instructions

>> 
>> diff --git a/libavformat/avio.c b/libavformat/avio.c
>> index 3606eb0..ecf6801 100644
>> --- a/libavformat/avio.c
>> +++ b/libavformat/avio.c
>> @@ -645,6 +645,13 @@ int ffurl_get_multi_file_handle(URLContext *h, int 
>> **handles, int *numhandles)
>> return h->prot->url_get_multi_file_handle(h, handles, numhandles);
>> }
>> 
>> +int ffurl_get_short_seek(URLContext *h)
>> +{
>> +if (!h->prot->url_get_short_seek)
> 
>> +return -1;
> 
> should be some AVERROR code

Fixed

>> +return h->prot->url_get_short_seek(h);
>> +}
>> +
>> int ffurl_shutdown(URLContext *h, int flags)
>> {
>> if (!h->prot->url_shutdown)
>> diff --git a/libavformat/avio.h b/libavformat/avio.h
>> index b1ce1d1..0480981 100644
>> --- a/libavformat/avio.h
>> +++ b/libavformat/avio.h
>> @@ -287,6 +287,12 @@ typedef struct AVIOContext {
>> int short_seek_threshold;
>> 
>> /**
>> + * A callback that is used instead of short_seek_threshold.
>> + * This is current internal only, do not use from outside.
>> + */
>> +int (*short_seek_get)(void *opaque);
>> +
>> +/**
>>  * ',' separated list of allowed protocols.
>>  */
>> const char *protocol_whitelist;
> 
> thats not ok to add this way, the docs say this:
> /**
> * Bytestream IO Context.
> * New fields can be added to the end with minor version bumps.
> * Removal, reordering and changes to existing fields require a major
> * version bump.
> * sizeof(AVIOContext) must not be used outside libav*.
> *
> * @note None of the function pointers in AVIOContext should be called
> *   directly, they should only be set by the client application
> *   when implementing custom I/O. Normally these are set to the
> *   function pointers specified in avio_alloc_context()
> */
> 

I moved short_seek_get to the end of AVIOContext.  I’m not sure if the minor 
version bump referenced in the comment requires any in-code changes.  Is this 
an acceptable magnitude of change for this feature?

> [...]
>> --- a/libavformat/tcp.c
>> +++ b/libavformat/tcp.c
>> @@ -265,6 +265,26 @@ static int tcp_get_file_handle(URLContext *h)
>> return s->fd;
>> }
>> 
>> +static int tcp_get_window_size(URLContext *h)
>> +{
>> +TCPContext *s = h->priv_data;
>> +int avail;
>> +int avail_len = sizeof(avail);
>> +
> 
>> +#if HAVE_WINSOCK2_H
> 
> this formating is IIRC not allowed in C the # must be in the first
> column
> 
> 
>> +/* SO_RCVBUF with winsock only reports the actual TCP window size when
>> +auto-tuning has been disabled via setting SO_RCVBUF */
>> +if (s->recv_buffer_size < 0) {
>> +return AVERROR(ENOSYS);
>> +}
>> +#endif
>> +
>> +if (getsockopt(s->fd, SOL_SOCKET, SO_RCVBUF, , _len)) {
>> +   return ff_neterrno();
>> +}
> 
> the indention is inconsisntent

Both formatting issues have been corrected

From bac0f351afd14c56c56376d20557b7330bcea23e Mon Sep 17 00:00:00 2001
From: Joel Cunningham 
Date: Fri, 13 Jan 2017 10:52:25 -0600
Subject: [PATCH] HTTP: improve performance by reducing forward seeks

This commit optimizes HTTP performance by reducing forward seeks, instead
favoring a read-ahead and discard on the current connection (referred to
as a short seek) for seeks that are within a TCP window's worth of data.
This improves performance because with TCP flow control, a window's worth
of data will be in the local socket buffer already or in-flight from the
sender once congestion control on the sender is fully utilizing the window.

Note: this approach doesn't attempt to differentiate from a newly opened
connection which may not be fully utilizing the window due to congestion
control vs one that is. The receiver can't get at this information, so we
assume worst case; that full window is in use (we did advertise it after all)
and that data could be in-flight

The previous behavior of closing the connection, then opening a new
with a new HTTP range value results in a massive amounts of discarded
and re-sent data when large TCP windows are used.  This has been observed
on MacOS/iOS which starts with an initial window of 256KB and grows up to
1MB depending on the bandwidth-product delay.

When seeking within a window's worth of data and we close 

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 06:43:45AM +0100, wm4 wrote:
> On Thu, 26 Jan 2017 03:20:02 +0100
> Michael Niedermayer  wrote:
> 
> > On Thu, Jan 26, 2017 at 02:58:07AM +0100, Andreas Cadhalpun wrote:
> > > On 26.01.2017 02:29, Ronald S. Bultje wrote:  
> > > > On Wed, Jan 25, 2017 at 8:12 PM, Andreas Cadhalpun <  
> > > > andreas.cadhal...@googlemail.com> wrote:  
> > > >   
> > > >> Signed-off-by: Andreas Cadhalpun 
> > > >> ---
> > > >>  libavformat/nistspheredec.c | 11 +++
> > > >>  1 file changed, 11 insertions(+)
> > > >>
> > > >> diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c
> > > >> index 782d1dfbfb..3386497682 100644
> > > >> --- a/libavformat/nistspheredec.c
> > > >> +++ b/libavformat/nistspheredec.c
> > > >> @@ -21,6 +21,7 @@
> > > >>
> > > >>  #include "libavutil/avstring.h"
> > > >>  #include "libavutil/intreadwrite.h"
> > > >> +#include "libavcodec/internal.h"
> > > >>  #include "avformat.h"
> > > >>  #include "internal.h"
> > > >>  #include "pcm.h"
> > > >> @@ -90,6 +91,11 @@ static int nist_read_header(AVFormatContext *s)
> > > >>  return 0;
> > > >>  } else if (!memcmp(buffer, "channel_count", 13)) {
> > > >>  sscanf(buffer, "%*s %*s %"SCNd32, 
> > > >> >codecpar->channels);
> > > >> +if (st->codecpar->channels > FF_SANE_NB_CHANNELS) {
> > > >> +av_log(s, AV_LOG_ERROR, "Too many channels %d > %d\n",
> > > >> +   st->codecpar->channels, FF_SANE_NB_CHANNELS);
> > > >> +return AVERROR(ENOSYS);
> > > >> +}  
> > > > 
> > > > 
> > > > I've said this before, but again - please don't add useless log 
> > > > messages.  
> > > 
> > > I disagree that these log messages are useless and I'm not the only one 
> > > [1].  
> > 
> > +1
> > 
> > Log messages make debuging the code easier, as a developer i like to
> > know why something fails having a hard failure but no clear and easy
> > findable error message is bad
> > 
> > 
> > [...]
> 
> -1
> 
> This kind of things bloat the code with rare corner cases. One point
> would be that this increases binary size (why do we even still have the
> NULL_IF_CONFIG_SMALL macro? I'm not saying we should use it here, but
> the current use of the macro will barely even make a dent into the
> number of bytes "wasted" for optional strings, yet we bother).
> 
> Another point is that code becomes unreadable if obscure corner cases
> take up most of the code. I think that's w worrisome direction we're
> taking here.

While it doesnt apply to this patch here but,
Error messages in obscure checks that describe the error condition
in the source would make at least some checks easier to understand
than just a generic
"return AVERROR_INVALIDDATA"



> 
> When I debug FFmpeg API use, the thing that annoys me most is that I
> can't know where an error happens. I have to trace the origin manually.
> Error codes are almost always completely useless. This patchset adds a
> lot of NOSYS, which is used 254 times in FFmpeg and which is about 99%
> meaningless and resolves to an even worse error message when using
> av_err2str(). Adding an av_log to error error point would "work" (at
> least you could use grep), but isn't a good solution.

I think the idea about something more informative than a int32 error
code did come up previously.
I certainly would be in favor of having an error value that could be
used to pinpoint the error location(s) and or function(s) it passed
through, this would be usefull for debguging in general

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

It is what and why we do it that matters, not just one of them.


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

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[wm4]

On Thu, 26 Jan 2017 14:29:21 +0100
Paul Arzelier  wrote:

> From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
> From: Polochon-street 
> Date: Thu, 26 Jan 2017 13:25:22 +0100
> Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis 
>  comments)
> Originally-by: Ben Boeckel 
> ---
>  libavformat/utils.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index d5dfca7dec..ce24244135 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
> *filename,
>  AVFormatContext *s = *ps;
>  int i, ret = 0;
>  AVDictionary *tmp = NULL;
> +AVDictionary *id3_meta = NULL;
>  ID3v2ExtraMeta *id3v2_extra_meta = NULL;
>  
>  if (!s && !(s = avformat_alloc_context()))
> @@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const 
> char *filename,
>  
>  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
>  if (s->pb)
> -ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
> +ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, 
> _extra_meta);
>  
>  if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)
>  if ((ret = s->iformat->read_header(s)) < 0)
>  goto fail;
>  
> +if (!s->metadata) {
> +s->metadata = id3_meta;
> +id3_meta = NULL;
> +} else if (id3_meta) {
> +int level = AV_LOG_WARNING;
> +if (s->error_recognition & AV_EF_COMPLIANT)
> +level = AV_LOG_ERROR;
> +av_log(s, level, "Discarding ID3 tag because more suitable tags were 
> found.\n");
> +av_dict_free(_meta);
> +if (s->error_recognition & AV_EF_EXPLODE)
> +return AVERROR_INVALIDDATA;
> +}
> +
>  if (id3v2_extra_meta) {
>  if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, 
> "aac") ||
>  !strcmp(s->iformat->name, "tta")) {
> @@ -627,6 +641,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
> *filename,
>  fail:
>  ff_id3v2_free_extra_meta(_extra_meta);
>  av_dict_free();
> +av_dict_free(_meta);
>  if (s->pb && !(s->flags & AVFMT_FLAG_CUSTOM_IO))
>  avio_closep(>pb);
>  avformat_free_context(s);

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

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[Paul Arzelier]

Alright, attached is the last version (I hope!)

Paul


Le 26/01/2017 à 13:43, wm4 a écrit :

On Thu, 26 Jan 2017 13:32:08 +0100
Paul Arzelier  wrote:


 From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
From: Polochon-street 
Date: Thu, 26 Jan 2017 13:25:22 +0100
Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis
  comments)
Originally-by: Ben Boeckel 
---
  libavformat/utils.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)


Patch looks generally great to me now.


diff --git a/libavformat/utils.c b/libavformat/utils.c
index d5dfca7dec..ce24244135 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
*filename,
  AVFormatContext *s = *ps;
  int i, ret = 0;
  AVDictionary *tmp = NULL;
+AVDictionary *id3_meta = NULL;
  ID3v2ExtraMeta *id3v2_extra_meta = NULL;
  
  if (!s && !(s = avformat_alloc_context()))

@@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const char 
*filename,
  
  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */

  if (s->pb)
-ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
+ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, 
_extra_meta);
  
  if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)

  if ((ret = s->iformat->read_header(s)) < 0)
  goto fail;
  
+if (!s->metadata) {

+av_dict_copy(>metadata, id3_meta, AV_DICT_DONT_OVERWRITE);
+av_dict_free(_meta);

Strictly speaking, this line is redundant, but it doesn't harm either.
If you feel like it you could remove it, but it's not necessary.


+}
+else if (id3_meta) {

If you make another patch, you could merge the } with the next line too
(I'm not sure, but I think that's preferred coding-style wise).


+int level = AV_LOG_WARNING;
+if (s->error_recognition & AV_EF_COMPLIANT)
+level = AV_LOG_ERROR;
+av_log(s, level, "Discarding ID3 tag because more suitable tags were 
found.\n");
+if (s->error_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;
+}
+
  if (id3v2_extra_meta) {
  if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, 
"aac") ||
  !strcmp(s->iformat->name, "tta")) {
@@ -627,6 +641,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
*filename,
  fail:
  ff_id3v2_free_extra_meta(_extra_meta);
  av_dict_free();
+   av_dict_free(_meta);
  if (s->pb && !(s->flags & AVFMT_FLAG_CUSTOM_IO))
  avio_closep(>pb);
  avformat_free_context(s);

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


--
Paul ARZELIER
Élève ingénieur à l'École Centrale de Lille
2014-2017

>From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
From: Polochon-street 
Date: Thu, 26 Jan 2017 13:25:22 +0100
Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis 
 comments)
Originally-by: Ben Boeckel 
---
 libavformat/utils.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index d5dfca7dec..ce24244135 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
 AVFormatContext *s = *ps;
 int i, ret = 0;
 AVDictionary *tmp = NULL;
+AVDictionary *id3_meta = NULL;
 ID3v2ExtraMeta *id3v2_extra_meta = NULL;
 
 if (!s && !(s = avformat_alloc_context()))
@@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
 
 /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
 if (s->pb)
-ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
+ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, _extra_meta);
 
 if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)
 if ((ret = s->iformat->read_header(s)) < 0)
 goto fail;
 
+if (!s->metadata) {
+s->metadata = id3_meta;
+id3_meta = NULL;
+} else if (id3_meta) {
+int level = AV_LOG_WARNING;
+if (s->error_recognition & AV_EF_COMPLIANT)
+level = AV_LOG_ERROR;
+av_log(s, level, "Discarding ID3 tag because more suitable tags were found.\n");
+av_dict_free(_meta);
+if (s->error_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;
+}
+
 if (id3v2_extra_meta) {
 if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, "aac") ||
 !strcmp(s->iformat->name, "tta")) {
@@ -627,6 +641,7 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
 fail:
 

Re: [FFmpeg-devel] [PATCH 1/3] mpeg12dec: validate color space

[Ronald S. Bultje]

Hi,

On Wed, Jan 25, 2017 at 8:56 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:

> On 26.01.2017 02:26, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Wed, Jan 25, 2017 at 8:20 PM, Andreas Cadhalpun <
> > andreas.cadhal...@googlemail.com> wrote:
> >
> >> On 07.01.2017 13:44, Ronald S. Bultje wrote:
> >>> Doesn't this destroy exporting of newly added colorspaces that have no
> >>> explicit mention yet in libavutil? I'm not 100% sure this is a good
> >> thing.
> >>
> >> Yes. That's how it is already done in libavcodec/h264_ps.c, so it just
> >> makes handling unknown values consistent.
> >
> >
> > Changing h264_ps.c would also make things consistent.
>
> No, because also libavcodec/options_table.h limits the colorspace to known
> values.


That is input only, this is output.

> The question is not whether changing A or B towards the other makes the
> > combination consistent. The question is which form of consistency is
> > better...
>
> As new values don't get added that often, I don't see a problem with
> requiring
> to explicitly add them to libavutil before being able to use them.


That is because your use case for libavcodec is constrained, you use it
only for decoding videos and fuzzing. There are others in this community
that do a lot more than that with libavcodec.

Look, Andreas, I appreciate the work you're doing, I really do, but you
always pick a fight with every review I put out on your code. I don't
understand why. My reviews are not difficult to address, they really are
not. My reviews are actionable, and if the action is taken, I'm happy and
the code can be committed. Why pick a fight? What is the point? Do you
think I'm an idiot that has no clue what he's doing and should be shot down
because of that? Please appreciate that I do have some clue of what I'm
doing, and I am looking out for the health of the project, just like you,
but with a slightly different perspective to some things. If I'm wrong,
please point it out, I make mistakes also, but in cases like these, it can
also help to just drop it and move on. Resolving an issue is not losing, it
is winning.

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

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[wm4]

On Thu, 26 Jan 2017 13:32:08 +0100
Paul Arzelier  wrote:

> From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
> From: Polochon-street 
> Date: Thu, 26 Jan 2017 13:25:22 +0100
> Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis 
>  comments)
> Originally-by: Ben Boeckel 
> ---
>  libavformat/utils.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 

Patch looks generally great to me now.

> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index d5dfca7dec..ce24244135 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
> *filename,
>  AVFormatContext *s = *ps;
>  int i, ret = 0;
>  AVDictionary *tmp = NULL;
> +AVDictionary *id3_meta = NULL;
>  ID3v2ExtraMeta *id3v2_extra_meta = NULL;
>  
>  if (!s && !(s = avformat_alloc_context()))
> @@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const 
> char *filename,
>  
>  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
>  if (s->pb)
> -ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
> +ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, 
> _extra_meta);
>  
>  if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)
>  if ((ret = s->iformat->read_header(s)) < 0)
>  goto fail;
>  
> +if (!s->metadata) {
> +av_dict_copy(>metadata, id3_meta, AV_DICT_DONT_OVERWRITE);

> +av_dict_free(_meta);

Strictly speaking, this line is redundant, but it doesn't harm either.
If you feel like it you could remove it, but it's not necessary.

> +}
> +else if (id3_meta) {

If you make another patch, you could merge the } with the next line too
(I'm not sure, but I think that's preferred coding-style wise).

> +int level = AV_LOG_WARNING;
> +if (s->error_recognition & AV_EF_COMPLIANT)
> +level = AV_LOG_ERROR;
> +av_log(s, level, "Discarding ID3 tag because more suitable tags were 
> found.\n");
> +if (s->error_recognition & AV_EF_EXPLODE)
> +return AVERROR_INVALIDDATA;
> +}
> +
>  if (id3v2_extra_meta) {
>  if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, 
> "aac") ||
>  !strcmp(s->iformat->name, "tta")) {
> @@ -627,6 +641,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
> *filename,
>  fail:
>  ff_id3v2_free_extra_meta(_extra_meta);
>  av_dict_free();
> +   av_dict_free(_meta);
>  if (s->pb && !(s->flags & AVFMT_FLAG_CUSTOM_IO))
>  avio_closep(>pb);
>  avformat_free_context(s);

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

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[Paul Arzelier]

Ah, missed the leaked, sorry.

Regarding the explode message, I kept it because of this message 
http://ffmpeg.org/pipermail/ffmpeg-devel/2015-February/168250.html . 
Maybe it is not that relevant anymore though, because we're talking 
about all existing codecs there, and not only FLAC (which seems to be a 
bit more strict when it comes down to tags).



Le 26/01/2017 à 13:10, wm4 a écrit :

On Thu, 26 Jan 2017 12:55:15 +0100
Paul Arzelier  wrote:


 From a3dc6068fb06722aacea56365f948afdb8df841f Mon Sep 17 00:00:00 2001
From: Paul Arzelier 
Date: Thu, 26 Jan 2017 12:51:33 +0100
Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis
  comments)
Originally-by: Ben Boeckel 
---
  libavformat/utils.c | 16 +++-
  1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index d5dfca7dec..bbe5c1ff1c 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
*filename,
  AVFormatContext *s = *ps;
  int i, ret = 0;
  AVDictionary *tmp = NULL;
+AVDictionary *id3_meta = NULL;

Looks like this would leak on "goto fail;".


  ID3v2ExtraMeta *id3v2_extra_meta = NULL;
  
  if (!s && !(s = avformat_alloc_context()))

@@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const char 
*filename,
  
  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */

  if (s->pb)
-ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
+ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, 
_extra_meta);
  
  if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)

  if ((ret = s->iformat->read_header(s)) < 0)
  goto fail;
  
+if (!s->metadata) {

+av_dict_copy(>metadata, id3_meta, AV_DICT_DONT_OVERWRITE);
+av_dict_free(_meta);
+}
+else if (id3_meta) {
+int level = AV_LOG_WARNING;
+if (s->error_recognition & AV_EF_COMPLIANT)
+level = AV_LOG_ERROR;
+av_log(s, level, "Spec-compliant flac files do not support ID3 
tags.\n");

Seems ok, but this message will be printed for non-flac formats too,
which is weird at best. Maybe something along the id3 tag being
discarded or so?


+if (s->error_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;

Not sure about this one, but I'd be fine with it personally.


+}
+
  if (id3v2_extra_meta) {
  if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, 
"aac") ||
  !strcmp(s->iformat->name, "tta")) {

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


--
Paul ARZELIER
Élève ingénieur à l'École Centrale de Lille
2014-2017

>From d84648e1990ad3a12462dfb76990dc7036f5f082 Mon Sep 17 00:00:00 2001
From: Polochon-street 
Date: Thu, 26 Jan 2017 13:25:22 +0100
Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis 
 comments)
Originally-by: Ben Boeckel 
---
 libavformat/utils.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index d5dfca7dec..ce24244135 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
 AVFormatContext *s = *ps;
 int i, ret = 0;
 AVDictionary *tmp = NULL;
+AVDictionary *id3_meta = NULL;
 ID3v2ExtraMeta *id3v2_extra_meta = NULL;
 
 if (!s && !(s = avformat_alloc_context()))
@@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
 
 /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
 if (s->pb)
-ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
+ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, _extra_meta);
 
 if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)
 if ((ret = s->iformat->read_header(s)) < 0)
 goto fail;
 
+if (!s->metadata) {
+av_dict_copy(>metadata, id3_meta, AV_DICT_DONT_OVERWRITE);
+av_dict_free(_meta);
+}
+else if (id3_meta) {
+int level = AV_LOG_WARNING;
+if (s->error_recognition & AV_EF_COMPLIANT)
+level = AV_LOG_ERROR;
+av_log(s, level, "Discarding ID3 tag because more suitable tags were found.\n");
+if (s->error_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;
+}
+
 if (id3v2_extra_meta) {
 if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, "aac") ||
 !strcmp(s->iformat->name, "tta")) {
@@ -627,6 +641,7 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
 fail:
 ff_id3v2_free_extra_meta(_extra_meta);
 av_dict_free();
+   

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[wm4]

On Thu, 26 Jan 2017 12:55:15 +0100
Paul Arzelier  wrote:

> From a3dc6068fb06722aacea56365f948afdb8df841f Mon Sep 17 00:00:00 2001
> From: Paul Arzelier 
> Date: Thu, 26 Jan 2017 12:51:33 +0100
> Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis
>  comments)
> Originally-by: Ben Boeckel 
> ---
>  libavformat/utils.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index d5dfca7dec..bbe5c1ff1c 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
> *filename,
>  AVFormatContext *s = *ps;
>  int i, ret = 0;
>  AVDictionary *tmp = NULL;
> +AVDictionary *id3_meta = NULL;

Looks like this would leak on "goto fail;".

>  ID3v2ExtraMeta *id3v2_extra_meta = NULL;
>  
>  if (!s && !(s = avformat_alloc_context()))
> @@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const 
> char *filename,
>  
>  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
>  if (s->pb)
> -ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
> +ff_id3v2_read_dict(s->pb, _meta, ID3v2_DEFAULT_MAGIC, 
> _extra_meta);
>  
>  if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)
>  if ((ret = s->iformat->read_header(s)) < 0)
>  goto fail;
>  
> +if (!s->metadata) {
> +av_dict_copy(>metadata, id3_meta, AV_DICT_DONT_OVERWRITE);
> +av_dict_free(_meta);
> +}
> +else if (id3_meta) {
> +int level = AV_LOG_WARNING;
> +if (s->error_recognition & AV_EF_COMPLIANT)
> +level = AV_LOG_ERROR;
> +av_log(s, level, "Spec-compliant flac files do not support ID3 
> tags.\n");

Seems ok, but this message will be printed for non-flac formats too,
which is weird at best. Maybe something along the id3 tag being
discarded or so?

> +if (s->error_recognition & AV_EF_EXPLODE)
> +return AVERROR_INVALIDDATA;

Not sure about this one, but I'd be fine with it personally.

> +}
> +
>  if (id3v2_extra_meta) {
>  if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, 
> "aac") ||
>  !strcmp(s->iformat->name, "tta")) {

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

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[Paul Arzelier]

You're right, I made a patch for libavformat/utils.c instead.

I modified a bit Ben's version and kept only the "spec-compliant flac 
files do not support ID3 tags" warning, I hope it's okay with you.


Paul

Le 26/01/2017 à 11:24, wm4 a écrit :

On Thu, 26 Jan 2017 11:08:45 +0100
Paul Arzelier  wrote:


 From bb5dc73c42ad2e0bffd7e38fac18c6f01ec05ab3 Mon Sep 17 00:00:00 2001
From: Paul Arzelier 
Date: Thu, 26 Jan 2017 11:04:44 +0100
Subject: [PATCH] Fixed behavior when id3 tags were found on FLAC files
Originally-by: Ben Boeckel 
---
  libavformat/flacdec.c | 30 ++
  1 file changed, 30 insertions(+)

diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
index 66baba5922..869936621b 100644
--- a/libavformat/flacdec.c
+++ b/libavformat/flacdec.c
@@ -48,6 +48,7 @@ static int flac_read_header(AVFormatContext *s)
  int ret, metadata_last=0, metadata_type, metadata_size, 
found_streaminfo=0;
  uint8_t header[4];
  uint8_t *buffer=NULL;
+int has_id3 = 0;
  FLACDecContext *flac = s->priv_data;
  AVStream *st = avformat_new_stream(s, NULL);
  if (!st)
@@ -63,6 +64,22 @@ static int flac_read_header(AVFormatContext *s)
  return 0;
  }
  
+if (av_dict_count(s->metadata)) {

+ /* XXX: Is there a better way to parse this out? ID3 parsing is done
+ * all the way out in avformat_open_input. */
+ has_id3 = 1;
+}
+
+if (has_id3) {
+ int level = AV_LOG_WARNING;
+ if (s->error_recognition & AV_EF_COMPLIANT)
+  level = AV_LOG_ERROR;
+ av_log(s, level, "Spec-compliant flac files do not support ID3 
tags.\n");
+ if (s->error_recognition & AV_EF_EXPLODE)
+  return AVERROR_INVALIDDATA;
+}
+
+
  /* process metadata blocks */
  while (!avio_feof(s->pb) && !metadata_last) {
  if (avio_read(s->pb, header, 4) != 4)
@@ -173,8 +190,16 @@ static int flac_read_header(AVFormatContext *s)
  }
  /* process supported blocks other than STREAMINFO */
  if (metadata_type == FLAC_METADATA_TYPE_VORBIS_COMMENT) {
+AVDictionary *other_meta = NULL;
  AVDictionaryEntry *chmask;
  
+if (has_id3) {

+av_log(s, AV_LOG_VERBOSE, "FLAC found with ID3 and vorbis tags; 
ignoring ID3 tags also provided by vorbis.\n");
+
+other_meta = s->metadata;
+s->metadata = NULL;
+}
+
  ret = ff_vorbis_comment(s, >metadata, buffer, 
metadata_size, 1);
  if (ret < 0) {
  av_log(s, AV_LOG_WARNING, "error parsing VorbisComment 
metadata\n");
@@ -182,6 +207,11 @@ static int flac_read_header(AVFormatContext *s)
  s->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
  }
  
+if (other_meta) {

+av_dict_copy(>metadata, other_meta, 
AV_DICT_DONT_OVERWRITE);
+av_dict_free(_meta);
+}
+
  /* parse the channels mask if present */
  chmask = av_dict_get(s->metadata, 
"WAVEFORMATEXTENSIBLE_CHANNEL_MASK", NULL, 0);
  if (chmask) {

I feel like there should be a more general solution.

ID3v2 tags are read by common code, and such tags are supported by
basically all file formats in libavformat due to the way it's
implemented.

This fixes it for FLAC only. Are we going to duplicate this for every
format?

I suggest reading id3 tags into a separate dict instead, and then
copying it in libavformat/utils.c after read_header if the demuxer
hasn't set any tags.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


--
Paul ARZELIER
Élève ingénieur à l'École Centrale de Lille
2014-2017

>From a3dc6068fb06722aacea56365f948afdb8df841f Mon Sep 17 00:00:00 2001
From: Paul Arzelier 
Date: Thu, 26 Jan 2017 12:51:33 +0100
Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis
 comments)
Originally-by: Ben Boeckel 
---
 libavformat/utils.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index d5dfca7dec..bbe5c1ff1c 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -513,6 +513,7 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
 AVFormatContext *s = *ps;
 int i, ret = 0;
 AVDictionary *tmp = NULL;
+AVDictionary *id3_meta = NULL;
 ID3v2ExtraMeta *id3v2_extra_meta = NULL;
 
 if (!s && !(s = avformat_alloc_context()))
@@ -588,12 +589,25 @@ int avformat_open_input(AVFormatContext **ps, const char *filename,
 
 /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */
 if (s->pb)
-ff_id3v2_read(s, 

Re: [FFmpeg-devel] [PATCH 2/2] avformat/udp: Replace use of pthread_cancel.

[Massimo Battistel]

>
>
> Ive reupped the current versions of each seperated patch for comment
> ___
>
>
Hi,
could this patch fix issue #5783? (https://trac.ffmpeg.org/ticket/5783)

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

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[wm4]

On Thu, 26 Jan 2017 11:08:45 +0100
Paul Arzelier  wrote:

> From bb5dc73c42ad2e0bffd7e38fac18c6f01ec05ab3 Mon Sep 17 00:00:00 2001
> From: Paul Arzelier 
> Date: Thu, 26 Jan 2017 11:04:44 +0100
> Subject: [PATCH] Fixed behavior when id3 tags were found on FLAC files
> Originally-by: Ben Boeckel 
> ---
>  libavformat/flacdec.c | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
> index 66baba5922..869936621b 100644
> --- a/libavformat/flacdec.c
> +++ b/libavformat/flacdec.c
> @@ -48,6 +48,7 @@ static int flac_read_header(AVFormatContext *s)
>  int ret, metadata_last=0, metadata_type, metadata_size, 
> found_streaminfo=0;
>  uint8_t header[4];
>  uint8_t *buffer=NULL;
> +int has_id3 = 0;
>  FLACDecContext *flac = s->priv_data;
>  AVStream *st = avformat_new_stream(s, NULL);
>  if (!st)
> @@ -63,6 +64,22 @@ static int flac_read_header(AVFormatContext *s)
>  return 0;
>  }
>  
> +if (av_dict_count(s->metadata)) {
> + /* XXX: Is there a better way to parse this out? ID3 parsing is done
> + * all the way out in avformat_open_input. */
> + has_id3 = 1;
> +}
> +
> +if (has_id3) {
> + int level = AV_LOG_WARNING;
> + if (s->error_recognition & AV_EF_COMPLIANT)
> +  level = AV_LOG_ERROR;
> + av_log(s, level, "Spec-compliant flac files do not support ID3 
> tags.\n");
> + if (s->error_recognition & AV_EF_EXPLODE)
> +  return AVERROR_INVALIDDATA;
> +}
> +
> +
>  /* process metadata blocks */
>  while (!avio_feof(s->pb) && !metadata_last) {
>  if (avio_read(s->pb, header, 4) != 4)
> @@ -173,8 +190,16 @@ static int flac_read_header(AVFormatContext *s)
>  }
>  /* process supported blocks other than STREAMINFO */
>  if (metadata_type == FLAC_METADATA_TYPE_VORBIS_COMMENT) {
> +AVDictionary *other_meta = NULL;
>  AVDictionaryEntry *chmask;
>  
> +if (has_id3) {
> +av_log(s, AV_LOG_VERBOSE, "FLAC found with ID3 and 
> vorbis tags; ignoring ID3 tags also provided by vorbis.\n");
> +
> +other_meta = s->metadata;
> +s->metadata = NULL;
> +}
> +
>  ret = ff_vorbis_comment(s, >metadata, buffer, 
> metadata_size, 1);
>  if (ret < 0) {
>  av_log(s, AV_LOG_WARNING, "error parsing VorbisComment 
> metadata\n");
> @@ -182,6 +207,11 @@ static int flac_read_header(AVFormatContext *s)
>  s->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
>  }
>  
> +if (other_meta) {
> +av_dict_copy(>metadata, other_meta, 
> AV_DICT_DONT_OVERWRITE);
> +av_dict_free(_meta);
> +}
> +
>  /* parse the channels mask if present */
>  chmask = av_dict_get(s->metadata, 
> "WAVEFORMATEXTENSIBLE_CHANNEL_MASK", NULL, 0);
>  if (chmask) {

I feel like there should be a more general solution.

ID3v2 tags are read by common code, and such tags are supported by
basically all file formats in libavformat due to the way it's
implemented.

This fixes it for FLAC only. Are we going to duplicate this for every
format?

I suggest reading id3 tags into a separate dict instead, and then
copying it in libavformat/utils.c after read_header if the demuxer
hasn't set any tags.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[Paul Arzelier]

Oops, I don't know what went wrong, but now it seems to work.

I've attached the working patched, sorry!


Le 26/01/2017 à 02:12, Michael Niedermayer a écrit :

On Wed, Jan 25, 2017 at 08:33:53PM +0100, Paul Arzelier wrote:

Hi all,

Would it be possible to continue the discussion began there ?
http://ffmpeg.org/pipermail/ffmpeg-devel/2015-February/168248.html

The current behavior of FFmpeg when it comes to manage FLAC tags with
both vorbis tags and ID3 tags is to append both, which leads to, for
example, "Artist: Pink Floyd;Pink Floyd" in FFprobe. This is not a
desired output: as the XIPH faq says about FLAC
(https://xiph.org/flac/faq.html):
"FLAC has it's own native tagging system which is identical to that of
Vorbis. They are called alternately "FLAC tags" and "Vorbis comments".
It is the only tagging system required and guaranteed to be supported by
FLAC implementations.
Out of convenience, the reference decoder knows how to skip ID3 tags so
that they don't interfere with decoding. But you should not expect any
tags beside FLAC tags to be supported in applications; some
implementations may not even be able to decode a FLAC file with ID3 tags."

This issue was also submitted here https://trac.ffmpeg.org/ticket/3799,
but still discuted in the above thread.
Ben Boeckel made the attached patch (which I edited a bit to fit FFmpeg
last version), which fixes the issue.
The problem that remained was that many other files could have
irrelevant id3v2 tags, not only Vorbis/FLAC files, so another more
global patch may be needed to fix that definitely.

So, do you think Ben's patch could be merged "as-is", or does it only
raise more questions?

Regards,
Paul


the attached patch is corrupted:

Applying: Fixed behavior when id3 tags were found on FLAC files
error: corrupt patch at line 58
Patch failed at 0001 Fixed behavior when id3 tags were found on FLAC files
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


[...]



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


--
Paul ARZELIER
Élève ingénieur à l'École Centrale de Lille
2014-2017

>From bb5dc73c42ad2e0bffd7e38fac18c6f01ec05ab3 Mon Sep 17 00:00:00 2001
From: Paul Arzelier 
Date: Thu, 26 Jan 2017 11:04:44 +0100
Subject: [PATCH] Fixed behavior when id3 tags were found on FLAC files
Originally-by: Ben Boeckel 
---
 libavformat/flacdec.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
index 66baba5922..869936621b 100644
--- a/libavformat/flacdec.c
+++ b/libavformat/flacdec.c
@@ -48,6 +48,7 @@ static int flac_read_header(AVFormatContext *s)
 int ret, metadata_last=0, metadata_type, metadata_size, found_streaminfo=0;
 uint8_t header[4];
 uint8_t *buffer=NULL;
+int has_id3 = 0;
 FLACDecContext *flac = s->priv_data;
 AVStream *st = avformat_new_stream(s, NULL);
 if (!st)
@@ -63,6 +64,22 @@ static int flac_read_header(AVFormatContext *s)
 return 0;
 }
 
+if (av_dict_count(s->metadata)) {
+ /* XXX: Is there a better way to parse this out? ID3 parsing is done
+ * all the way out in avformat_open_input. */
+ has_id3 = 1;
+}
+
+if (has_id3) {
+ int level = AV_LOG_WARNING;
+ if (s->error_recognition & AV_EF_COMPLIANT)
+  level = AV_LOG_ERROR;
+ av_log(s, level, "Spec-compliant flac files do not support ID3 tags.\n");
+ if (s->error_recognition & AV_EF_EXPLODE)
+  return AVERROR_INVALIDDATA;
+}
+
+
 /* process metadata blocks */
 while (!avio_feof(s->pb) && !metadata_last) {
 if (avio_read(s->pb, header, 4) != 4)
@@ -173,8 +190,16 @@ static int flac_read_header(AVFormatContext *s)
 }
 /* process supported blocks other than STREAMINFO */
 if (metadata_type == FLAC_METADATA_TYPE_VORBIS_COMMENT) {
+AVDictionary *other_meta = NULL;
 AVDictionaryEntry *chmask;
 
+if (has_id3) {
+av_log(s, AV_LOG_VERBOSE, "FLAC found with ID3 and vorbis tags; ignoring ID3 tags also provided by vorbis.\n");
+
+other_meta = s->metadata;
+s->metadata = NULL;
+}
+
 ret = ff_vorbis_comment(s, >metadata, buffer, metadata_size, 1);
 if (ret < 0) {
 av_log(s, AV_LOG_WARNING, "error parsing VorbisComment metadata\n");
@@ -182,6 +207,11 @@ static int flac_read_header(AVFormatContext *s)
 s->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
 }
 
+if 

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Paul B Mahol]

On 1/26/17, wm4  wrote:
> On Thu, 26 Jan 2017 03:20:02 +0100
> Michael Niedermayer  wrote:
>
>> On Thu, Jan 26, 2017 at 02:58:07AM +0100, Andreas Cadhalpun wrote:
>> > On 26.01.2017 02:29, Ronald S. Bultje wrote:
>> > > On Wed, Jan 25, 2017 at 8:12 PM, Andreas Cadhalpun <
>> > > andreas.cadhal...@googlemail.com> wrote:
>> > >
>> > >> Signed-off-by: Andreas Cadhalpun 
>> > >> ---
>> > >>  libavformat/nistspheredec.c | 11 +++
>> > >>  1 file changed, 11 insertions(+)
>> > >>
>> > >> diff --git a/libavformat/nistspheredec.c
>> > >> b/libavformat/nistspheredec.c
>> > >> index 782d1dfbfb..3386497682 100644
>> > >> --- a/libavformat/nistspheredec.c
>> > >> +++ b/libavformat/nistspheredec.c
>> > >> @@ -21,6 +21,7 @@
>> > >>
>> > >>  #include "libavutil/avstring.h"
>> > >>  #include "libavutil/intreadwrite.h"
>> > >> +#include "libavcodec/internal.h"
>> > >>  #include "avformat.h"
>> > >>  #include "internal.h"
>> > >>  #include "pcm.h"
>> > >> @@ -90,6 +91,11 @@ static int nist_read_header(AVFormatContext *s)
>> > >>  return 0;
>> > >>  } else if (!memcmp(buffer, "channel_count", 13)) {
>> > >>  sscanf(buffer, "%*s %*s %"SCNd32,
>> > >> >codecpar->channels);
>> > >> +if (st->codecpar->channels > FF_SANE_NB_CHANNELS) {
>> > >> +av_log(s, AV_LOG_ERROR, "Too many channels %d >
>> > >> %d\n",
>> > >> +   st->codecpar->channels, FF_SANE_NB_CHANNELS);
>> > >> +return AVERROR(ENOSYS);
>> > >> +}
>> > >
>> > >
>> > > I've said this before, but again - please don't add useless log
>> > > messages.
>> >
>> > I disagree that these log messages are useless and I'm not the only one
>> > [1].
>>
>> +1
>>
>> Log messages make debuging the code easier, as a developer i like to
>> know why something fails having a hard failure but no clear and easy
>> findable error message is bad
>>
>>
>> [...]
>
> -1
>
> This kind of things bloat the code with rare corner cases. One point
> would be that this increases binary size (why do we even still have the
> NULL_IF_CONFIG_SMALL macro? I'm not saying we should use it here, but
> the current use of the macro will barely even make a dent into the
> number of bytes "wasted" for optional strings, yet we bother).
>
> Another point is that code becomes unreadable if obscure corner cases
> take up most of the code. I think that's w worrisome direction we're
> taking here.
>
> When I debug FFmpeg API use, the thing that annoys me most is that I
> can't know where an error happens. I have to trace the origin manually.
> Error codes are almost always completely useless. This patchset adds a
> lot of NOSYS, which is used 254 times in FFmpeg and which is about 99%
> meaningless and resolves to an even worse error message when using
> av_err2str(). Adding an av_log to error error point would "work" (at
> least you could use grep), but isn't a good solution.
>
> In this particular case, I'd question why the code calling this
> function (avformat_open_inputavcodec_open) doesn't print an error
> message instead, or so.
>
> But of course not every API user wants such an error message (but still
> wants others).


I don't want those log messages in there.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 6/9] pvfdec: prevent overflow during block alignment calculation

[Paul B Mahol]

On 1/26/17, Andreas Cadhalpun  wrote:
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/pvfdec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>

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

Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata

[wm4]

On Thu, 26 Jan 2017 09:16:00 +0100
Carl Eugen Hoyos  wrote:

> 2017-01-26 9:07 GMT+01:00 wm4 :
> 
> >> >> > Any metadata you export can and will get copied to a new file when
> >> >> > remuxing, therefor exporting arbitrary info that isn't actual stream
> >> >> > metadata tags in metadata is problematic - it carries over to the
> >> >> > destination file, in which it would be entirely meaningless.  
> >> >>
> >> >> Sorry, I don't understand:
> >> >> Which application do you mean?  
> >> >
> >> > ffmpeg  
> >>
> >> Didn't I ping yesterday a patch that avoids this?
> >> And wasn't this the mail you answered?  
> >
> > Sorry, I couldn't find it just now. What was the subject line?  
> 
> My mailer (gmail) shows it as the first mail of this thread, so
> it (hopefully) uses the same subject.

Sorry, I missed that. But this code is not called for remuxing, or is
it?

> > I'm not necessarily blocking your patch.  
> 
> > There are already plenty of other cases where it breaks this
> > way,  
> 
> Where did you report this?
> I believe you know how difficult it is to fix bugs that do not get
> reported.

I think I mentioned it multiple times.

I do not report ffmpeg bugs because I'm more productive not doing that,
sorry.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] libavformat patch that brute-forces aax encryption

[wm4]

On Tue, 24 Jan 2017 19:48:00 -0700
William Shipley  wrote:

> I made a small modification of libavformat that bruteforces the 4-byte code
> used in audible encrypted files. It automatically runs if an aax is passed
> (always encrypted) without the code provided. Previously, it would tell the
> user the code was needed and exit.
> 
> It takes between 5 and 10 minutes to crack it as currently implemented,
> upon which it performs the specified task (conversion, content extraction,
> etc) and outputs the decryption key on the console.
> 
> Is there any interest in including this upstream? If it's a code quality
> issue, I'm open to suggestions, but if it's felt that this is outside the
> scope of the project or legally risky then I understand.
> 
> I didn't do any kind of reverse engineering or anything legally gray as far
> as I know, just noticed that it's literally a 32-bit key after the fixed
> key is in place (which was already in ffmpeg code). I used a legally
> obtained aax from my own audible account to test it, even.
> 
> The key it outputs is the same key you get from tools like
> audible-activator. It's basically a user ID for a login.
> 
> I currently have a fork up on github here:
> https://github.com/FFmpeg/FFmpeg/compare/master...willrandship:master
> I'll generate a patch file if you're interested.

I'm fairly sure this is not really appropriate to put into a demuxer.
Especially if it means that opening a file can hang for 5 to 10 minutes
eating 100% CPU.

It should probably be a separate file. (Could even be in FFmpeg's
tools/ directory, so I'm not necessarily rejecting it for this project.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata

[Carl Eugen Hoyos]

2017-01-26 9:07 GMT+01:00 wm4 :

>> >> > Any metadata you export can and will get copied to a new file when
>> >> > remuxing, therefor exporting arbitrary info that isn't actual stream
>> >> > metadata tags in metadata is problematic - it carries over to the
>> >> > destination file, in which it would be entirely meaningless.
>> >>
>> >> Sorry, I don't understand:
>> >> Which application do you mean?
>> >
>> > ffmpeg
>>
>> Didn't I ping yesterday a patch that avoids this?
>> And wasn't this the mail you answered?
>
> Sorry, I couldn't find it just now. What was the subject line?

My mailer (gmail) shows it as the first mail of this thread, so
it (hopefully) uses the same subject.

> I'm not necessarily blocking your patch.

> There are already plenty of other cases where it breaks this
> way,

Where did you report this?
I believe you know how difficult it is to fix bugs that do not get
reported.

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

Re: [FFmpeg-devel] [PATCH]lavf/mov: Ignore avio_skip() return value

[wm4]

On Thu, 26 Jan 2017 08:25:15 +0100
Carl Eugen Hoyos  wrote:

> 2017-01-26 6:27 GMT+01:00 wm4 :
> > On Thu, 26 Jan 2017 00:34:20 +0100
> > Carl Eugen Hoyos  wrote:
> >  
> >> From 694daed9222e50d6245bf5d041e82523ee869451 Mon Sep 17 00:00:00 2001
> >> From: Carl Eugen Hoyos 
> >> Date: Thu, 26 Jan 2017 00:32:23 +0100
> >> Subject: [PATCH] lavf/mov: Ignore avio_skip() return value.
> >>
> >> Fixes ticket #6102.
> >> ---
> >>  libavformat/mov.c |4 +---
> >>  1 file changed, 1 insertion(+), 3 deletions(-)
> >>
> >> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >> index 7dc550e..6f7e80a 100644
> >> --- a/libavformat/mov.c
> >> +++ b/libavformat/mov.c
> >> @@ -4794,9 +4794,7 @@ static int mov_read_uuid(MOVContext *c, AVIOContext 
> >> *pb, MOVAtom atom)
> >>  av_free(buffer);
> >>  } else {
> >>  // skip all uuid atom, which makes it fast for long uuid-xmp 
> >> file
> >> -ret = avio_skip(pb, len);
> >> -if (ret < 0)
> >> -return ret;
> >> +avio_skip(pb, len);
> >>  }
> >>  } else if (!memcmp(uuid, uuid_spherical, sizeof(uuid))) {
> >>  size_t len = atom.size - sizeof(uuid);  
> >
> > Why would you just ignore the error?  
> 
> It fixes a regression (that I suspect will hit you very soon) and
> it's what all other (17? 24?) calls to avio_skip() in mov.c do.
> 
> Doesn't an error here indicate a hardware issue?

We do not normally ignore errors, especially not hardware errors.

mov.c really does contain a whole lot of unchecked skip, read and seek
calls. Low code quality, I guess.

Anyway, in issue #6102 you mentioned that this was caused by this
commit:

http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=25e35b34365ea4fc737f406992b7947a0610edcb

The original code had no unchecked calls. Are you sure the commit
shouldn't just reverted, instead of removing an error check? Can you
explain why it's ok to ignore the error in this case specifically? Does
it actually work, or just prevent read_header from returning early?

Also, please provide all the information in the commit message, instead
of just mentioning an issue. This is why the commit message field
exists; use it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] libavformat patch that brute-forces aax encryption

[Carl Eugen Hoyos]

2017-01-25 3:48 GMT+01:00 William Shipley :

> Is there any interest in including this upstream?

Please send your patch to this mailing list.

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

Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata

[wm4]

On Thu, 26 Jan 2017 08:26:02 +0100
Carl Eugen Hoyos  wrote:

> 2017-01-26 6:24 GMT+01:00 wm4 :
> > On Thu, 26 Jan 2017 00:35:17 +0100
> > Carl Eugen Hoyos  wrote:
> >  
> >> 2017-01-26 0:19 GMT+01:00 Hendrik Leppkes :  
> >> > On Thu, Jan 26, 2017 at 10:04 AM, Carl Eugen Hoyos  
> >> > wrote:  
> >> >> 2017-01-25 14:22 GMT+01:00 wm4 :  
> >> >>> On Mon, 12 Dec 2016 12:12:37 +0100
> >> >>> Carl Eugen Hoyos  wrote:
> >> >>>  
> >>  From 7c26220a8734fe7dc293efe6c13e3baf91defc7e Mon Sep 17 00:00:00 2001
> >>  From: Carl Eugen Hoyos 
> >>  Date: Mon, 12 Dec 2016 12:07:27 +0100
> >>  Subject: [PATCH 2/2] lavf/mov: Export vendor metadata.
> >> 
> >>  ---
> >>   libavformat/mov.c |5 -
> >>   tests/ref/fate/mov-zombie |2 +-
> >>   2 files changed, 5 insertions(+), 2 deletions(-)
> >> 
> >>  diff --git a/libavformat/mov.c b/libavformat/mov.c
> >>  index 0b1c182..a19ebbf 100644
> >>  --- a/libavformat/mov.c
> >>  +++ b/libavformat/mov.c
> >>  @@ -1842,6 +1842,7 @@ static void mov_parse_stsd_video(MOVContext *c, 
> >>  AVIOContext *pb,
> >>    AVStream *st, MOVStreamContext *sc)
> >>   {
> >>   uint8_t codec_name[32] = { 0 };
> >>  +uint8_t vendor[5] = { 0 };
> >>   int64_t stsd_start;
> >>   unsigned int len;
> >> 
> >>  @@ -1851,7 +1852,9 @@ static void mov_parse_stsd_video(MOVContext *c, 
> >>  AVIOContext *pb,
> >> 
> >>   avio_rb16(pb); /* version */
> >>   avio_rb16(pb); /* revision level */
> >>  -avio_rb32(pb); /* vendor */
> >>  +avio_read(pb, vendor, 4);
> >>  +if (vendor[0])
> >>  +av_dict_set(>metadata, "vendor", vendor, 0);  
> >> >>>
> >> >>> Does this mean transcoding to a format with per-stream
> >> >>> tags will add this as a tag?  
> >> >>
> >> >> The patch you quoted allows libavformat users to read the
> >> >> vendor tag from mov files, I don't understand how it adds
> >> >> tags.
> >> >> Do you object?  
> >> >
> >> > Any metadata you export can and will get copied to a new file when
> >> > remuxing, therefor exporting arbitrary info that isn't actual stream
> >> > metadata tags in metadata is problematic - it carries over to the
> >> > destination file, in which it would be entirely meaningless.  
> >>
> >> Sorry, I don't understand:
> >> Which application do you mean?  
> >
> > ffmpeg  
> 
> Didn't I ping yesterday a patch that avoids this?
> And wasn't this the mail you answered?

Sorry, I couldn't find it just now. What was the subject line?

I'm not necessarily blocking your patch. There are already plenty of
other cases where it breaks this way, and your patch just adds another.
But maybe they can all be fixed by that patch you mentioned.

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