Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata
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(&st->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?
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
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?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation
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, &st->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).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata
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(&st->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
Or does it somehow have a whitelist of which tags to copy? I didn't
attempt to confirm the behavior.
___
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
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?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation
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, &st->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.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation
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, &st->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.
Maybe some middle ground can be found defining macros so the user can
decide if he wants these messages in the build or not, also maybe some
"probability" levels can be added to these errors so the macros can work
based on them, and finally a lot of similar checks exist around the
codebase which could be factorized, so the code - even with reported
messages - would look less cluttered with functionally useless parts.
Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/segment: remove last_cut check when detecting a new segment
Not starting a new segment if the elapsed microsecs since the start of the day
equals the the elapsed microsecs since the start of the day at the time of the
last cut seems plain wrong to me, Deti do you remember the original reason
behind this check?
Signed-off-by: Marton Balint
---
libavformat/segment.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/libavformat/segment.c b/libavformat/segment.c
index 9d47148..8ec3653 100644
--- a/libavformat/segment.c
+++ b/libavformat/segment.c
@@ -87,7 +87,6 @@ typedef struct SegmentContext {
int64_t clocktime_offset; //< clock offset for cutting the segments at
regular clock time
int64_t clocktime_wrap_duration; //< wrapping duration considered for
starting a new segment
int64_t last_val; ///< remember last time for wrap around detection
-int64_t last_cut; ///< remember last cut
int cut_pending;
int header_written;///< whether we've already called
avformat_write_header
@@ -870,10 +869,8 @@ calc_times:
localtime_r(&sec, &ti);
usecs = (int64_t)(ti.tm_hour * 3600 + ti.tm_min * 60 + ti.tm_sec)
* 100 + (avgt % 100);
wrapped_val = (usecs + seg->clocktime_offset) % seg->time;
-if (seg->last_cut != usecs && wrapped_val < seg->last_val &&
wrapped_val < seg->clocktime_wrap_duration) {
+if (wrapped_val < seg->last_val && wrapped_val <
seg->clocktime_wrap_duration)
seg->cut_pending = 1;
-seg->last_cut = usecs;
-}
seg->last_val = wrapped_val;
} else {
end_pts = seg->time * (seg->segment_count + 1);
--
2.10.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/9] 4xm: prevent overflow during block alignment calculation
On Thu, Jan 26, 2017 at 02:11:02AM +0100, Andreas Cadhalpun wrote: > Signed-off-by: Andreas Cadhalpun > --- > libavformat/4xm.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation
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, &st->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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Avoid a single point of failure, be that a person or equipment.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation
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, &st->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].
> These don't help end users at all, since these samples are exceedingly
> unlikely to be real.
Files can get corrupted randomly, so I think this error isn't less likely
than most other errors.
> Most likely, they derive from fuzzing, and stderr
> cramming is one of the things that makes fuzzing slower.
Printing log messages in inner decoding loops makes that slower, but a
log message during header parsing is hardly noticeable.
Best regards,
Andreas
1: https://ffmpeg.org/pipermail/ffmpeg-devel/2017-January/205433.html
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] mpeg12dec: validate color space
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. > 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. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] HTTP: optimize forward seek performance
On Fri, Jan 13, 2017 at 11:36:41AM -0600, Joel Cunningham wrote: > > > On Jan 12, 2017, at 10:59 AM, Joel Cunningham > > wrote: > > > > Nicolas, > > > > I’ve found existing “read-ahead logic” in avio_seek to do what I’ve > > implemented in http_stream_forward(). This is controlled by > > SHORT_SEEK_THRESHOLD, currently set to 4KB. I proto-typed increasing this > > to the 256KB (matches the initial TCP window in my test setup) and saw the > > same number of reduction in HTTP GETs and the number of seeks! Thanks for > > the heads up, this should reduce my patch size! > > > > I could plumb this setting (s->short_seek_threshold) to a URL function that > > would get the desired value from HTTP/TCP. Looking at how > > ffio_init_context() is implemented, it doesn’t appear to have access to the > > URLContext pointer. Any guidance on how I can plumb the protocol layer > > with aviobuf without a public API change? > > > > Thanks, > > > > Joel > > > I managed to figure the plumbing out in a way that doesn’t modify any public > APIs. I added a new callback to struct AVIOContext that gets a short seek > threshold value from URLContext if available (otherwise fallback to > s->short_seek_threshold). This allows using the read-ahead/discard logic in > avio_seek and eliminates my forwarding logic in the HTTP layer (thanks to > Nicolas for pointing me in this direction). See new patch below > > I also updated my commit message to include assumptions/considerations about > congestion control on the sender (thanks to Andy for calling out the > discussion on it). Lastly, I have upload new captures/log in dropbox for > those that want to take a look at my testing output (see > ffplay_short_seek_output.log and mac-ffplay-short-seek-patch.pcapng) > > https://www.dropbox.com/sh/4q4ru8isdv22joj/AABU3XyXmgLMiEFqucf1LdZ3a?dl=0 > > Thanks for the feedback so far, > > Joel > > From 9bb2f184591c2d6e6a91d3760e63b013ca4c95e5 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 inital 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 the connection, > then open a new one within the same window's worth of data, we discard > from the current offset till the end of the window. Then on the new > connection the server ends up re-sending the previous data from new > offset till the end of old window. > > Example (assumes full window utilization): > > TCP window size: 64KB > Position: 32KB > Forward seek position: 40KB > > * (Next window) > 32KB |--| 96KB |---| 160KB > * > 40KB |---| 104KB > > Re-sent amount: 96KB - 40KB = 56KB > > For a real world test example, I have MP4 file of ~25MB, which ffplay > only reads ~16MB and performs 177 seeks. With current ffmpeg, this results > in 177 HTTP GETs and ~73MB worth of TCP data communication. With this > patch, ffmpeg issues 4 HTTP GETs and 3 seeks for a total of ~22MB of TCP data > communication. > > To support this feature, the short seek logic in avio_seek() has been > extended to call a function to get the short seek threshold value. This > callback has been plumbed to the URLProtocol structure, which now has > infrastructure in HTTP and TCP to get the underlying receiver window size > via SO_RCVBUF. If the underlying URL and protocol don't support returning > a short seek threshold, the default s->short_seek_threshold is used > > This feature has been tested on Windows 7 and MacOS/iOS. Windows support > is slightly complicated by the fact that when TCP window auto-tuning is > enabled, SO_RCVBUF doesn't report the real window size, but it does if > SO_RCVBUF was manually set (disabling auto-tuning). So we can only use > this optimization
Re: [FFmpeg-devel] [PATCH 1/3] mpeg12dec: validate color space
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:
> > On Fri, Jan 6, 2017 at 9:35 PM, Michael Niedermayer
>
> > wrote:
> >
> >> On Fri, Jan 06, 2017 at 09:43:24PM +0100, Andreas Cadhalpun wrote:
> >>> On 23.12.2016 00:57, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun
> ---
> libavcodec/mpeg12dec.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index 63979079c8..d3dc67ad6a 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -1470,6 +1470,10 @@ static void mpeg_decode_sequence_display_
> extension(Mpeg1Context
> >> *s1)
> s->avctx->color_primaries = get_bits(&s->gb, 8);
> s->avctx->color_trc = get_bits(&s->gb, 8);
> s->avctx->colorspace = get_bits(&s->gb, 8);
> +if (!av_color_space_name(s->avctx->colorspace)) {
> +av_log(s->avctx, AV_LOG_WARNING, "Invalid color space %d,
> >> setting to unspecified\n", s->avctx->colorspace);
> +s->avctx->colorspace = AVCOL_SPC_UNSPECIFIED;
> +}
> }
> w = get_bits(&s->gb, 14);
> skip_bits(&s->gb, 1); // marker
>
> >>>
> >>> Ping for the series.
> >>
> >> i have no real objection to it.
> >> iam used to see these being exported unchanged though so it feels a
> >> bit odd
> >
> >
> > 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.
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...
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation
Hi,
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, &st->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.
These don't help end users at all, since these samples are exceedingly
unlikely to be real. Most likely, they derive from fuzzing, and stderr
cramming is one of the things that makes fuzzing slower.
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] mpeg12dec: validate color space
On 07.01.2017 13:44, Ronald S. Bultje wrote:
> On Fri, Jan 6, 2017 at 9:35 PM, Michael Niedermayer
> wrote:
>
>> On Fri, Jan 06, 2017 at 09:43:24PM +0100, Andreas Cadhalpun wrote:
>>> On 23.12.2016 00:57, Andreas Cadhalpun wrote:
Signed-off-by: Andreas Cadhalpun
---
libavcodec/mpeg12dec.c | 4
1 file changed, 4 insertions(+)
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 63979079c8..d3dc67ad6a 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1470,6 +1470,10 @@ static void
mpeg_decode_sequence_display_extension(Mpeg1Context
>> *s1)
s->avctx->color_primaries = get_bits(&s->gb, 8);
s->avctx->color_trc = get_bits(&s->gb, 8);
s->avctx->colorspace = get_bits(&s->gb, 8);
+if (!av_color_space_name(s->avctx->colorspace)) {
+av_log(s->avctx, AV_LOG_WARNING, "Invalid color space %d,
>> setting to unspecified\n", s->avctx->colorspace);
+s->avctx->colorspace = AVCOL_SPC_UNSPECIFIED;
+}
}
w = get_bits(&s->gb, 14);
skip_bits(&s->gb, 1); // marker
>>>
>>> Ping for the series.
>>
>> i have no real objection to it.
>> iam used to see these being exported unchanged though so it feels a
>> bit odd
>
>
> 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.
Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 8/9] xvag: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun
---
libavformat/xvag.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavformat/xvag.c b/libavformat/xvag.c
index 5ef4fb0900..22e4f1e3c8 100644
--- a/libavformat/xvag.c
+++ b/libavformat/xvag.c
@@ -20,6 +20,7 @@
*/
#include "libavutil/bswap.h"
+#include "libavcodec/internal.h"
#include "avformat.h"
#include "internal.h"
@@ -68,7 +69,7 @@ static int xvag_read_header(AVFormatContext *s)
if (st->codecpar->sample_rate <= 0)
return AVERROR_INVALIDDATA;
-if (st->codecpar->channels <= 0)
+if (st->codecpar->channels <= 0 || st->codecpar->channels >
FF_SANE_NB_CHANNELS)
return AVERROR_INVALIDDATA;
switch (codec) {
--
2.11.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 9/9] boadec: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun
---
libavformat/boadec.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/libavformat/boadec.c b/libavformat/boadec.c
index ac2a33b3f0..6055effcad 100644
--- a/libavformat/boadec.c
+++ b/libavformat/boadec.c
@@ -20,6 +20,7 @@
*/
#include "libavutil/intreadwrite.h"
+#include "libavcodec/internal.h"
#include "avformat.h"
#include "internal.h"
@@ -53,9 +54,20 @@ static int read_header(AVFormatContext *s)
avio_rl32(s->pb);
st->codecpar->sample_rate = avio_rl32(s->pb);
st->codecpar->channels= avio_rl32(s->pb);
+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);
+}
s->internal->data_offset = avio_rl32(s->pb);
avio_r8(s->pb);
-st->codecpar->block_align = st->codecpar->channels * avio_rl32(s->pb);
+st->codecpar->block_align = avio_rl32(s->pb);
+if (st->codecpar->block_align > INT_MAX / FF_SANE_NB_CHANNELS) {
+av_log(s, AV_LOG_ERROR, "Too large block alignment %d > %d\n",
+ st->codecpar->block_align, INT_MAX / FF_SANE_NB_CHANNELS);
+return AVERROR_INVALIDDATA;
+}
+st->codecpar->block_align *= st->codecpar->channels;
avio_seek(s->pb, s->internal->data_offset, SEEK_SET);
--
2.11.0
___
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
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". [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 7/9] epafdec: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun --- libavformat/epafdec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/epafdec.c b/libavformat/epafdec.c index 29190fff72..0cd9627a4b 100644 --- a/libavformat/epafdec.c +++ b/libavformat/epafdec.c @@ -20,6 +20,7 @@ */ #include "libavutil/intreadwrite.h" +#include "libavcodec/internal.h" #include "avformat.h" #include "internal.h" #include "pcm.h" @@ -59,7 +60,7 @@ static int epaf_read_header(AVFormatContext *s) channels= avio_rb32(s->pb); } -if (!channels || !sample_rate) +if (channels <= 0 || channels > FF_SANE_NB_CHANNELS || sample_rate <= 0) return AVERROR_INVALIDDATA; st = avformat_new_stream(s, NULL); -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 6/9] pvfdec: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun --- libavformat/pvfdec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavformat/pvfdec.c b/libavformat/pvfdec.c index b9f6d4f2c2..c6652b9b43 100644 --- a/libavformat/pvfdec.c +++ b/libavformat/pvfdec.c @@ -19,6 +19,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ +#include "libavcodec/internal.h" #include "avformat.h" #include "internal.h" #include "pcm.h" @@ -44,7 +45,8 @@ static int pvf_read_header(AVFormatContext *s) &bps) != 3) return AVERROR_INVALIDDATA; -if (channels <= 0 || bps <= 0 || sample_rate <= 0) +if (channels <= 0 || channels > FF_SANE_NB_CHANNELS || +bps <= 0 || bps > INT_MAX / FF_SANE_NB_CHANNELS || sample_rate <= 0) return AVERROR_INVALIDDATA; st = avformat_new_stream(s, NULL); -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation
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, &st->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);
+}
} else if (!memcmp(buffer, "sample_byte_format", 18)) {
sscanf(buffer, "%*s %*s %31s", format);
@@ -109,6 +115,11 @@ static int nist_read_header(AVFormatContext *s)
sscanf(buffer, "%*s %*s %"SCNd64, &st->duration);
} else if (!memcmp(buffer, "sample_n_bytes", 14)) {
sscanf(buffer, "%*s %*s %"SCNd32, &bps);
+if (bps > (INT_MAX / FF_SANE_NB_CHANNELS) >> 3) {
+av_log(s, AV_LOG_ERROR, "Too many bytes per sample %d > %d\n",
+ bps, (INT_MAX / FF_SANE_NB_CHANNELS) >> 3);
+return AVERROR_INVALIDDATA;
+}
} else if (!memcmp(buffer, "sample_rate", 11)) {
sscanf(buffer, "%*s %*s %"SCNd32, &st->codecpar->sample_rate);
} else if (!memcmp(buffer, "sample_sig_bits", 15)) {
--
2.11.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/9] ircamdec: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun
---
libavformat/ircamdec.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/libavformat/ircamdec.c b/libavformat/ircamdec.c
index 59f3a49411..e3196db84a 100644
--- a/libavformat/ircamdec.c
+++ b/libavformat/ircamdec.c
@@ -20,6 +20,7 @@
*/
#include "libavutil/intreadwrite.h"
+#include "libavcodec/internal.h"
#include "avformat.h"
#include "internal.h"
#include "pcm.h"
@@ -87,6 +88,11 @@ static int ircam_read_header(AVFormatContext *s)
st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
st->codecpar->channels= 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);
+}
st->codecpar->sample_rate = sample_rate;
st->codecpar->codec_id = ff_codec_get_id(tags, tag);
--
2.11.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] ffmpeg.c: Add output file index and stream index to vstats file.
Signed-off-by: Sasi Inguva
---
doc/ffmpeg.texi | 8 +++-
ffmpeg.c| 9 +++--
ffmpeg_opt.c| 2 +-
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index cdea1a271f..996d6a6cc4 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -637,9 +637,15 @@ Dump video coding statistics to @file{vstats_HHMMSS.log}.
@item -vstats_file @var{file}
Dump video coding statistics to @var{file}.
@item -vstats_version @var{file}
-Specifies which version of the vstats format to use. If version is 1, format is
+Specifies which version of the vstats format to use. Default is 2.
+
+version = 1 :
@code{frame= %5d q= %2.1f PSNR= %6.2f f_size= %6d s_size= %8.0fkB time= %0.3f
br= %7.1fkbits/s avg_br= %7.1fkbits/s}
+
+version > 1:
+
+@code{out= %2d st= %2d frame= %5d q= %2.1f PSNR= %6.2f f_size= %6d s_size=
%8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s}
@item -top[:@var{stream_specifier}] @var{n} (@emph{output,per-stream})
top=1/bottom=0/auto=-1 field first
@item -dc @var{precision}
diff --git a/ffmpeg.c b/ffmpeg.c
index 977708c069..0a8f448995 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -1347,8 +1347,13 @@ static void do_video_stats(OutputStream *ost, int
frame_size)
enc = ost->enc_ctx;
if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
frame_number = ost->st->nb_frames;
-fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
-ost->quality / (float)FF_QP2LAMBDA);
+if (vstats_version <= 1) {
+fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
+ost->quality / (float)FF_QP2LAMBDA);
+} else {
+fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ",
ost->file_index, ost->index, frame_number,
+ost->quality / (float)FF_QP2LAMBDA);
+}
if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] /
(enc->width * enc->height * 255.0 * 255.0)));
diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
index b1a62e8620..6a47d32b53 100644
--- a/ffmpeg_opt.c
+++ b/ffmpeg_opt.c
@@ -121,7 +121,7 @@ int frame_bits_per_raw_sample = 0;
float max_error_rate = 2.0/3;
int filter_nbthreads = 0;
int filter_complex_nbthreads = 0;
-int vstats_version = 1;
+int vstats_version = 2;
static int intra_only = 0;
--
2.11.0.483.g087da7b7c-goog
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/9] genh: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun --- libavformat/genh.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/genh.c b/libavformat/genh.c index b683e026d1..dd4e76d3d9 100644 --- a/libavformat/genh.c +++ b/libavformat/genh.c @@ -20,6 +20,7 @@ */ #include "libavutil/intreadwrite.h" +#include "libavcodec/internal.h" #include "avformat.h" #include "internal.h" @@ -54,7 +55,7 @@ static int genh_read_header(AVFormatContext *s) st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; st->codecpar->channels= avio_rl32(s->pb); -if (st->codecpar->channels <= 0) +if (st->codecpar->channels <= 0 || st->codecpar->channels > FF_SANE_NB_CHANNELS) return AVERROR_INVALIDDATA; if (st->codecpar->channels == 1) st->codecpar->channel_layout = AV_CH_LAYOUT_MONO; -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/9] electronicarts: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun
---
libavformat/electronicarts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
index 30eb723bd5..bfd3fed3a2 100644
--- a/libavformat/electronicarts.c
+++ b/libavformat/electronicarts.c
@@ -539,7 +539,7 @@ static int ea_read_header(AVFormatContext *s)
ea->audio_codec = 0;
return 1;
}
-if (ea->bytes <= 0) {
+if (ea->bytes <= 0 || ea->bytes > 2) {
av_log(s, AV_LOG_ERROR,
"Invalid number of bytes per sample: %d\n", ea->bytes);
ea->audio_codec = AV_CODEC_ID_NONE;
--
2.11.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/9] 4xm: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun
---
libavformat/4xm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/libavformat/4xm.c b/libavformat/4xm.c
index 2758b69d29..ead6d2b424 100644
--- a/libavformat/4xm.c
+++ b/libavformat/4xm.c
@@ -29,6 +29,7 @@
#include "libavutil/intreadwrite.h"
#include "libavutil/intfloat.h"
+#include "libavcodec/internal.h"
#include "avformat.h"
#include "internal.h"
@@ -153,8 +154,10 @@ static int parse_strk(AVFormatContext *s,
fourxm->tracks[track].audio_pts = 0;
if (fourxm->tracks[track].channels<= 0 ||
+fourxm->tracks[track].channels > FF_SANE_NB_CHANNELS ||
fourxm->tracks[track].sample_rate <= 0 ||
-fourxm->tracks[track].bits<= 0) {
+fourxm->tracks[track].bits<= 0 ||
+fourxm->tracks[track].bits > INT_MAX / FF_SANE_NB_CHANNELS) {
av_log(s, AV_LOG_ERROR, "audio header invalid\n");
return AVERROR_INVALIDDATA;
}
--
2.11.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/9] electronicarts: prevent overflow during block alignment calculation
On 07.01.2017 09:32, Paul B Mahol wrote: > On 1/7/17, Michael Niedermayer wrote: >> On Fri, Jan 06, 2017 at 08:47:39PM +0100, Andreas Cadhalpun wrote: >>> Signed-off-by: Andreas Cadhalpun >>> --- >>> libavformat/electronicarts.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c >>> index 30eb723bd5..03422e5b2c 100644 >>> --- a/libavformat/electronicarts.c >>> +++ b/libavformat/electronicarts.c >>> @@ -556,6 +556,7 @@ static int ea_read_header(AVFormatContext *s) >>> st->codecpar->codec_tag = 0; /* no tag */ >>> st->codecpar->channels = ea->num_channels; >>> st->codecpar->sample_rate = ea->sample_rate; >>> +FF_RETURN_ON_OVERFLOW(s, ea->bytes > INT_MAX / 8 / 2) >> >> I think we should ask for a sample when the number of bytes per >> sample is larger than 2 or 4 or whatever max we think occurs. > > No we should not as such samples are invalid. The code seems to only know about 1 (8-bit) and 2 (16-bit), so returning an error for larger values makes sense. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/9] 4xm: prevent overflow during block alignment calculation
On 07.01.2017 02:31, Michael Niedermayer wrote: > On Fri, Jan 06, 2017 at 09:27:29PM +0100, Andreas Cadhalpun wrote: >> 4xm.c |1 + >> 1 file changed, 1 insertion(+) >> 4b27cb10f25865014fac1666956f7040d65113f9 >> 0002-4xm-prevent-overflow-during-block-alignment-calculat.patch >> From 861b62eec30feaa56b10eec7ba4029daf48a3c28 Mon Sep 17 00:00:00 2001 >> From: Andreas Cadhalpun >> Date: Thu, 15 Dec 2016 02:14:31 +0100 >> Subject: [PATCH 2/9] 4xm: prevent overflow during block alignment calculation >> >> Signed-off-by: Andreas Cadhalpun >> --- >> libavformat/4xm.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/libavformat/4xm.c b/libavformat/4xm.c >> index 2758b69d29..58729fed0d 100644 >> --- a/libavformat/4xm.c >> +++ b/libavformat/4xm.c >> @@ -187,6 +187,7 @@ static int parse_strk(AVFormatContext *s, >> st->codecpar->bit_rate = (int64_t)st->codecpar->channels * >>st->codecpar->sample_rate * >> >> st->codecpar->bits_per_coded_sample; >> +FF_RETURN_ON_OVERFLOW(s, st->codecpar->bits_per_coded_sample > INT_MAX >> / st->codecpar->channels) >> st->codecpar->block_align = st->codecpar->channels * >> >> st->codecpar->bits_per_coded_sample; > > i think we should check channels for > 8 or something and ask for a > sample and check bits_per_coded_sample against what maximal sensible > value of bits a sample and ask for a sample if above Actually avcodec_open2 already errors out if channels is larger than FF_SANE_NB_CHANNELS = 64. That check can already be done in the demuxer. Then defining INT_MAX / 64 as maximal sensible value of bits_per_coded_sample eliminates the need for FF_RETURN_ON_OVERFLOW checks. I'll send an updated patch series. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] h264_ps: validate chroma sample location
On 07.01.2017 01:41, Michael Niedermayer wrote:
> On Fri, Jan 06, 2017 at 11:33:13PM +0100, Andreas Cadhalpun wrote:
>> On 06.01.2017 22:47, Kieran Kunhya wrote:
>>> On Fri, 6 Jan 2017 at 20:44 Andreas Cadhalpun <
>>> andreas.cadhal...@googlemail.com> wrote:
>>>
Signed-off-by: Andreas Cadhalpun
---
libavcodec/h264_ps.c | 4
1 file changed, 4 insertions(+)
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index 8218e3a010..089bfc650a 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -181,6 +181,10 @@ static inline int decode_vui_parameters(GetBitContext
*gb, AVCodecContext *avctx
if (get_bits1(gb)) {
/* chroma_sample_location_type_top_field */
avctx->chroma_sample_location = get_ue_golomb(gb) + 1;
+if (!av_chroma_location_name(avctx->chroma_sample_location)) {
+av_log(avctx, AV_LOG_WARNING, "Invalid chroma sample location
%d, setting to unspecified\n", avctx->chroma_sample_location);
+avctx->chroma_sample_location = AVCHROMA_LOC_UNSPECIFIED;
+}
>>> Is there a way to long only once, this seems like it could spam the user
>>> full of these warnings.
>>
>> One could add a field like shown_chroma_loc_warning to SPS, but I think
>> that would be a bit too much overhead for this.
>> Alternatively, one could drop the log message entirely. (Wrong color
>> primaries etc. aren't logged either...)
>
> I think making it a "normally not displayed" log level would be a
> better choice than removing it entirely
OK, I reduced it to AV_LOG_VERBOSE.
Best regards,
Andreas
>From 5874739904fa8f13be03faee27e4bb2ac061258f Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun
Date: Fri, 6 Jan 2017 21:36:39 +0100
Subject: [PATCH] h264_ps: validate chroma sample location
Signed-off-by: Andreas Cadhalpun
---
libavcodec/h264_ps.c | 4
1 file changed, 4 insertions(+)
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index 8218e3a010..474eaf0d2d 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -181,6 +181,10 @@ static inline int decode_vui_parameters(GetBitContext *gb, AVCodecContext *avctx
if (get_bits1(gb)) {
/* chroma_sample_location_type_top_field */
avctx->chroma_sample_location = get_ue_golomb(gb) + 1;
+if (!av_chroma_location_name(avctx->chroma_sample_location)) {
+av_log(avctx, AV_LOG_VERBOSE, "Invalid chroma sample location %d, setting to unspecified\n", avctx->chroma_sample_location);
+avctx->chroma_sample_location = AVCHROMA_LOC_UNSPECIFIED;
+}
get_ue_golomb(gb); /* chroma_sample_location_type_bottom_field */
}
--
2.11.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg.c: Add output file index and stream index to vstats file.
Updated the patch with using vstats_version. PTAL.
On Sat, Jan 21, 2017 at 3:52 AM, Michael Niedermayer
wrote:
> On Fri, Jan 20, 2017 at 07:31:09PM -0800, Sasi Inguva wrote:
> > Couldn't find any version that relates to vstats. There is nothing that
> > says format may change any time , but there is no defined format either.
> > Let me know the version enum I have to update if I need to.
>
> i dont think theres a enum
> I think the way to resolve this is to send a patch that updates the
> docs and either state that vstats format can change at any time or
> that it cannot in which case something like a -vtstats_version
> parameter would need to be added
>
> When the docs patch passes review, that is everyone is happy with the
> solution it describes then the change with or without a version
> check which either way then matches the docs can be pushed
>
> thx
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Dictatorship: All citizens are under surveillance, all their steps and
> actions recorded, for the politicians to enforce control.
> Democracy: All politicians are under surveillance, all their steps and
> actions recorded, for the citizens to enforce control.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
From 9bc4d038af3478467eddc45a634b0e03e163dc39 Mon Sep 17 00:00:00 2001
From: Sasi Inguva
Date: Thu, 19 Jan 2017 14:33:32 -0800
Subject: [PATCH] ffmpeg.c: Add output file index and stream index to vstats
file.
Signed-off-by: Sasi Inguva
---
doc/ffmpeg.texi | 8 +++-
ffmpeg.c| 9 +++--
ffmpeg_opt.c| 2 +-
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index cdea1a271f..996d6a6cc4 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -637,9 +637,15 @@ Dump video coding statistics to @file{vstats_HHMMSS.log}.
@item -vstats_file @var{file}
Dump video coding statistics to @var{file}.
@item -vstats_version @var{file}
-Specifies which version of the vstats format to use. If version is 1, format is
+Specifies which version of the vstats format to use. Default is 2.
+
+version = 1 :
@code{frame= %5d q= %2.1f PSNR= %6.2f f_size= %6d s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s}
+
+version > 1:
+
+@code{out= %2d st= %2d frame= %5d q= %2.1f PSNR= %6.2f f_size= %6d s_size= %8.0fkB time= %0.3f br= %7.1fkbits/s avg_br= %7.1fkbits/s}
@item -top[:@var{stream_specifier}] @var{n} (@emph{output,per-stream})
top=1/bottom=0/auto=-1 field first
@item -dc @var{precision}
diff --git a/ffmpeg.c b/ffmpeg.c
index 977708c069..0a8f448995 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -1347,8 +1347,13 @@ static void do_video_stats(OutputStream *ost, int frame_size)
enc = ost->enc_ctx;
if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
frame_number = ost->st->nb_frames;
-fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
-ost->quality / (float)FF_QP2LAMBDA);
+if (vstats_version <= 1) {
+fprintf(vstats_file, "frame= %5d q= %2.1f ", frame_number,
+ost->quality / (float)FF_QP2LAMBDA);
+} else {
+fprintf(vstats_file, "out= %2d st= %2d frame= %5d q= %2.1f ", ost->file_index, ost->index, frame_number,
+ost->quality / (float)FF_QP2LAMBDA);
+}
if (ost->error[0]>=0 && (enc->flags & AV_CODEC_FLAG_PSNR))
fprintf(vstats_file, "PSNR= %6.2f ", psnr(ost->error[0] / (enc->width * enc->height * 255.0 * 255.0)));
diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
index b1a62e8620..6a47d32b53 100644
--- a/ffmpeg_opt.c
+++ b/ffmpeg_opt.c
@@ -121,7 +121,7 @@ int frame_bits_per_raw_sample = 0;
float max_error_rate = 2.0/3;
int filter_nbthreads = 0;
int filter_complex_nbthreads = 0;
-int vstats_version = 1;
+int vstats_version = 2;
static int intra_only = 0;
--
2.11.0.483.g087da7b7c-goog
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] libavformat patch that brute-forces aax encryption
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. Thanks, William Shipley ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata
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(&st->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?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]lavf/mov: Ignore avio_skip() return value
Hi!
Attached patch fixes ticket #6102.
Please comment, Carl Eugen
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);
--
1.7.10.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata
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(&st->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.
- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata
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(&st->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?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg_opt.c: Introduce a -vstats_version option and document the existing -vstats format.
On Tue, Jan 24, 2017 at 08:23:54AM -0800, Sasi Inguva wrote: > Signed-off-by: Sasi Inguva > --- > doc/ffmpeg.texi | 4 > ffmpeg.h| 1 + > ffmpeg_opt.c| 3 +++ > 3 files changed, 8 insertions(+) applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No snowflake in an avalanche ever feels responsible. -- Voltaire signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] avformat/avienc: add reserve_index_space option
On Mon, Jan 23, 2017 at 11:12:13AM +0100, Tobias Rapp wrote: [...] > libavformat/avi.h |1 > libavformat/avienc.c| 77 > +--- > libavformat/version.h |2 > tests/ref/fate/mpeg4-bsf-unpack-bframes |2 > tests/ref/lavf-fate/avi_cram|2 > 5 files changed, 74 insertions(+), 10 deletions(-) > 8dfa5b4ff26ee67c6772bb257a772672bb91aa34 > 0002-avformat-avienc-add-reserve_index_space-option.patch > From 4cc70ffdeb7eeb60e7bbb725bd5885dcacf968d2 Mon Sep 17 00:00:00 2001 > From: Tobias Rapp > Date: Wed, 4 Jan 2017 15:31:29 +0100 > Subject: [PATCH 2/3] avformat/avienc: add reserve_index_space option > > Allows the user to reserve space for the ODML master index. A sufficient > sized master index in the AVI header avoids storing follow-up master > indexes within the 'movi' data later. > > If the option is omitted or zero the index size is estimated from output > duration and bitrate. A worst-case bitrate for video streams is assumed > in case it is not available. > > Note: fate reference files changed because the video stream had zero > bitrate before and is guessed now. I dont think this is good if the user app says bitrate is 0 the muxer should not replace that by the bitrate for a rawvideo stream (when its not rawvideo) This could be ok for the reserved space calculation but for the file bitrate, especially with lossy compressed streams this will likely give values that are less correct than before [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you think the mosad wants you dead since a long time then you are either wrong or dead since a long time. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] fate: add SCC test
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
diff --git a/tests/fate/subtitles.mak b/tests/fate/subtitles.mak
index 8aa0279..cc9d77c 100644
--- a/tests/fate/subtitles.mak
+++ b/tests/fate/subtitles.mak
@@ -94,6 +94,9 @@ fate-sub-textenc: CMD = fmtstdout srt -i
$(TARGET_SAMPLES)/sub/SubRip_capability
FATE_SUBTITLES_ASS-$(call ALLYES, MICRODVD_DEMUXER MICRODVD_DECODER ICONV) +=
fate-sub-charenc
fate-sub-charenc: CMD = fmtstdout ass -sub_charenc cp1251 -i
$(TARGET_SAMPLES)/sub/cp1251-subtitles.sub
+FATE_SUBTITLES-$(call ALLYES, SCC_DEMUXER CCAPTION_DECODER SUBRIP_ENCODER
SRT_MUXER) += fate-sub-scc
+fate-sub-scc: CMD = fmtstdout srt -i $(TARGET_SAMPLES)/sub/witch.scc
+
FATE_SUBTITLES-$(call ENCMUX, ASS, ASS) += $(FATE_SUBTITLES_ASS-yes)
FATE_SUBTITLES += $(FATE_SUBTITLES-yes)
diff --git a/tests/ref/fate/sub-scc b/tests/ref/fate/sub-scc
new file mode 100644
index 000..1c3ec05
--- /dev/null
+++ b/tests/ref/fate/sub-scc
@@ -0,0 +1,519 @@
+1
+00:00:00,000 --> 00:00:09,297
+{\an7}♪ PIES IESU DOMINE,
+\hDONNA EIS REQUIEM ♪
+\h\h\h\h\h\h\h\h\h\h\h\h\h\h\h\h[ Bangs head
+
+2
+00:00:10,561 --> 00:00:15,231
+{\an7}♪ PIES IESU DOMINE, ♪ [
+
+3
+00:00:22,627 --> 00:00:27,429
+{\an7}♪ PIES IESU DOMINE, ♪ [
+
+4
+00:00:28,231 --> 00:00:31,627
+{\an7}♪ DONNA EIS REQUIEM.♪ [
+
+5
+00:00:31,627 --> 00:00:36,594
+{\an7}[ Crowd Shouting ]
+A WITCH ! A WITCH ! A WITCH !
+♪ PIES IESU DOMINE, ♪ [
+
+6
+00:00:36,594 --> 00:00:39,363
+{\an7}A WITCH ! A WITCH !
+WE FOUND A WITCH !
+
+7
+00:00:39,363 --> 00:00:41,924
+{\an7}[ Crowd Continues
+\h\hShouting ]
+A WITCH ! A WITCH !
+
+8
+00:00:41,924 --> 00:00:45,660
+{\an7}WE’VE GOT A WITCH !
+A WITCH ! WE’VE GOT A WITCH !
+
+9
+00:00:45,660 --> 00:00:47,858
+{\an7}BURN HER !
+\hBURN HER ! BURN HER !
+
+10
+00:00:47,858 --> 00:00:50,891
+{\an7}WE’VE FOUND A WITCH !
+\hWE’VE FOUND A WITCH !
+
+11
+00:00:50,891 --> 00:00:54,528
+{\an7}A WITCH !
+\hA WITCH ! A WITCH !
+
+12
+00:00:54,528 --> 00:00:57,693
+{\an7}WE HAVE FOUND A WITCH !
+MAY WE BURN HER ?
+
+13
+00:00:57,693 --> 00:01:00,297
+{\an7}[ Crowd ]
+BURN HER ! BURN HER !
+
+14
+00:01:00,297 --> 00:01:04,066
+{\an7}HOW DO YOU KNOW
+SHE IS A WITCH ?
+SHE LOOKS LIKE ONE !
+
+15
+00:01:04,066 --> 00:01:06,264
+{\an7}[ Shouting
+\h\hAffirmations ]
+
+16
+00:01:06,264 --> 00:01:08,066
+{\an7}BRING HER FORWARD.
+
+17
+00:01:08,066 --> 00:01:11,264
+{\an7}I’M NOT A WITCH.
+\hI’M NOT A WITCH.
+
+18
+00:01:11,264 --> 00:01:13,033
+{\an7}BUT YOU ARE DRESSED
+AS ONE.
+
+19
+00:01:13,033 --> 00:01:16,033
+{\an7}THEY DRESSED ME UP
+LIKE THIS.
+\h\h\h\h\h\h\h\hNO ! WE DIDN’T !
+
+20
+00:01:16,033 --> 00:01:19,957
+{\an7}AND THIS ISN’T MY NOSE.
+IT’S A FALSE ONE.
+
+21
+00:01:22,297 --> 00:01:25,429
+{\an7}WELL ?
+\h\h\h\h\h\h\h\hWELL, WE DID
+\h\h\h\h\h\h\h\hDO THE NOSE.
+
+22
+00:01:25,429 --> 00:01:27,066
+{\an7}THE NOSE ?
+\h\h\h\h\h\h\h\hAND THE HAT.
+
+23
+00:01:27,066 --> 00:01:32,495
+{\an7}BUT SHE IS A WITCH.
+YEAH ! BURN HER !
+BURN HER ! BURN HER !
+
+24
+00:01:32,495 --> 00:01:35,462
+{\an7}- DID YOU DRESS HER UP
+\h\hLIKE THIS ?
+- NO !
+
+25
+00:01:35,462 --> 00:01:36,858
+{\an7}\h\h\h\h\h\h\h\hNO !
+NO !
+\h\h\h\hNO !
+
+26
+00:01:36,858 --> 00:01:38,627
+{\an7}YES. YES.
+YEAH, A BIT.
+
+27
+00:01:38,627 --> 00:01:41,825
+{\an7}\h\h\h\h\h\h\h\hA BIT.
+\h\h\h\h\h\h\h\h\hA BIT.
+SHE HAS GOT A WART.
+
+28
+00:01:41,825 --> 00:01:44,033
+{\an7}WHAT MAKES YOU THINK
+SHE IS A WITCH ?
+
+29
+00:01:44,033 --> 00:01:46,528
+{\an7}WELL, SHE TURNED ME
+INTO A NEWT !
+
+30
+00:01:46,528 --> 00:01:49,297
+{\an7}A NEWT ?
+
+31
+00:01:52,462 --> 00:01:54,396
+{\an7}I GOT BETTER.
+
+32
+00:01:54,396 --> 00:01:55,891
+{\an7}BURN HER ANYWAY !
+
+33
+00:01:55,891 --> 00:01:58,033
+{\an7}BURN HER !
+\h\h\h\h\h\h\h\hBURN HER !
+\h\h\h\h\h\h\h\hCRACKLE, CRACKLE !
+
+34
+00:01:58,033 --> 00:02:01,000
+{\an7}\h\h\h\h[ Shouting
+\h\h\h\h\h\hContinues ]
+QUIET. QUIET.
+
+35
+00:02:01,000 --> 00:02:02,429
+{\an7}QUIET !
+QUIET !
+
+36
+00:02:02,429 --> 00:02:06,132
+{\an7}THERE ARE WAYS OF TELLING
+WHETHER SHE IS A WITCH.
+
+37
+00:02:06,132 --> 00:02:07,957
+{\an7}ARE THERE ?
+WHAT ARE THEY ?
+
+38
+00:02:07,957 --> 00:02:10,297
+{\an7}TELL US !
+TELL US !
+\h\h\h\hDO THEY HURT ?
+
+39
+00:02:10,297 --> 00:02:13,231
+{\an7}TELL ME,
+WHAT DO YOU DO WITH WITCHES ?
+
+40
+00:02:13,231 --> 00:02:16,198
+{\an7}\h\h\h\h\h\h\h\h\h\h\h\hBURN THEM !
+\h\h\h\h\h\h\h\h\h\h\h\h\hBURN THEM UP !
+BURN !
+
+41
+00:02:16,198 --> 00:02:19,330
+{\an7}AND WHAT DO YOU BURN,
+APART FROM WITCHES ?
+
+42
+00:02:19,330 --> 00:02:21,594
+{\an7}MORE WITCHES !
+SHH !
+
+43
+00:02:21,594 --> 00:02:22,627
+{\an7}WOOD !
+
+44
+00:02:22,627 --> 00:02:26,330
+{\an7}SO,
+WHY DO WITCHES BURN ?
+
+45
+00:02:29,693 --> 00:02:32,033
+{\an7}B--
+
+46
+00:02:36,132 --> 00:02:39,462
+{\a
[FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist
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
>From b10a32fecf05b17f6e824b662b7db07bff195d2d Mon Sep 17 00:00:00 2001
From: Paul Arzelier
Date: Wed, 25 Jan 2017 18:55:52 +0100
Subject: [PATCH] Fixed behavior when id3 tags were found on FLAC files
Originally-by: Ben Boeckel
---
libavformat/flacdec.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
index 3060dc45fd..f8d4611398 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) {
avio_read(s->pb, header, 4);
@@ -172,8 +189,17 @@ 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, &s->metadata, buffer, metadata_size, 1);
if (ret < 0) {
av_log(s, AV_LOG_WARNING, "error parsing VorbisComment metadata\n");
@@ -181,6 +207,11 @@ static int flac_read_header(AVFormatContext *s)
s->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
}
+if (other_meta) {
+av_dict_copy(&s->metadata, other_meta, AV_DICT_DONT_OVERWRITE);
+av_dict_free(&other_meta);
+}
+
/* parse the channels mask if present */
chmask = av_dict_get(s->metadata, "WAVEFORMATEXTENSIBLE_CHANNEL_MASK", NULL, 0);
if (chmask) {
--
2.11.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec: add ATRAC Advanced Lossless decoders
Only lossy part is decoded for now.
Signed-off-by: Paul B Mahol
---
libavcodec/Makefile| 3 +
libavcodec/allcodecs.c | 2 +
libavcodec/atrac3.c| 134 ++-
libavcodec/atrac3plusdec.c | 14 +++-
libavcodec/avcodec.h | 2 +
libavcodec/codec_desc.c| 14
libavformat/oma.c | 10 +--
libavformat/oma.h | 2 +
libavformat/omadec.c | 173 -
9 files changed, 313 insertions(+), 41 deletions(-)
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 43a6add..c5505db 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -191,8 +191,11 @@ OBJS-$(CONFIG_ASV2_DECODER)+= asvdec.o asv.o
mpeg12data.o
OBJS-$(CONFIG_ASV2_ENCODER)+= asvenc.o asv.o mpeg12data.o
OBJS-$(CONFIG_ATRAC1_DECODER) += atrac1.o atrac.o
OBJS-$(CONFIG_ATRAC3_DECODER) += atrac3.o atrac.o
+OBJS-$(CONFIG_ATRAC3AL_DECODER)+= atrac3.o atrac.o
OBJS-$(CONFIG_ATRAC3P_DECODER) += atrac3plusdec.o atrac3plus.o \
atrac3plusdsp.o atrac.o
+OBJS-$(CONFIG_ATRAC3PAL_DECODER) += atrac3plusdec.o atrac3plus.o \
+ atrac3plusdsp.o atrac.o
OBJS-$(CONFIG_AURA_DECODER)+= cyuv.o
OBJS-$(CONFIG_AURA2_DECODER) += aura.o
OBJS-$(CONFIG_AVRN_DECODER)+= avrndec.o mjpegdec.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index f92b2b7..7220864 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -403,7 +403,9 @@ void avcodec_register_all(void)
REGISTER_DECODER(APE, ape);
REGISTER_DECODER(ATRAC1,atrac1);
REGISTER_DECODER(ATRAC3,atrac3);
+REGISTER_DECODER(ATRAC3AL, atrac3al);
REGISTER_DECODER(ATRAC3P, atrac3p);
+REGISTER_DECODER(ATRAC3PAL, atrac3pal);
REGISTER_DECODER(BINKAUDIO_DCT, binkaudio_dct);
REGISTER_DECODER(BINKAUDIO_RDFT,binkaudio_rdft);
REGISTER_DECODER(BMV_AUDIO, bmv_audio);
diff --git a/libavcodec/atrac3.c b/libavcodec/atrac3.c
index ffd93e4..5b8ede7 100644
--- a/libavcodec/atrac3.c
+++ b/libavcodec/atrac3.c
@@ -731,6 +731,101 @@ static int decode_frame(AVCodecContext *avctx, const
uint8_t *databuf,
return 0;
}
+static int al_decode_frame(AVCodecContext *avctx, const uint8_t *databuf,
+ int size, float **out_samples)
+{
+ATRAC3Context *q = avctx->priv_data;
+int ret, i;
+uint8_t *ptr1;
+
+if (q->coding_mode == JOINT_STEREO) {
+/* channel coupling mode */
+/* decode Sound Unit 1 */
+ret = init_get_bits8(&q->gb, databuf, size);
+if (ret < 0)
+return ret;
+
+ret = decode_channel_sound_unit(q, &q->gb, q->units, out_samples[0], 0,
+JOINT_STEREO);
+if (ret != 0)
+return ret;
+
+/* Framedata of the su2 in the joint-stereo mode is encoded in
+ * reverse byte order so we need to swap it first. */
+if (databuf == q->decoded_bytes_buffer) {
+uint8_t *ptr2 = q->decoded_bytes_buffer + size - 1;
+ptr1 = q->decoded_bytes_buffer;
+for (i = 0; i < size / 2; i++, ptr1++, ptr2--)
+FFSWAP(uint8_t, *ptr1, *ptr2);
+} else {
+const uint8_t *ptr2 = databuf + size - 1;
+for (i = 0; i < size; i++)
+q->decoded_bytes_buffer[i] = *ptr2--;
+}
+
+/* Skip the sync codes (0xF8). */
+ptr1 = q->decoded_bytes_buffer;
+for (i = 4; *ptr1 == 0xF8; i++, ptr1++) {
+if (i >= size)
+return AVERROR_INVALIDDATA;
+}
+
+
+/* set the bitstream reader at the start of the second Sound Unit*/
+init_get_bits8(&q->gb, ptr1, q->decoded_bytes_buffer + size - ptr1);
+
+/* Fill the Weighting coeffs delay buffer */
+memmove(q->weighting_delay, &q->weighting_delay[2],
+4 * sizeof(*q->weighting_delay));
+q->weighting_delay[4] = get_bits1(&q->gb);
+q->weighting_delay[5] = get_bits(&q->gb, 3);
+
+for (i = 0; i < 4; i++) {
+q->matrix_coeff_index_prev[i] = q->matrix_coeff_index_now[i];
+q->matrix_coeff_index_now[i] = q->matrix_coeff_index_next[i];
+q->matrix_coeff_index_next[i] = get_bits(&q->gb, 2);
+}
+
+/* Decode Sound Unit 2. */
+ret = decode_channel_sound_unit(q, &q->gb, &q->units[1],
+out_samples[1], 1, JOINT_STEREO);
+if (ret != 0)
+return ret;
+
+/* Reconstruct the channel coefficients. */
+reverse_matrixing(out_samples[0], out_samples[1],
+ q->matrix_coeff_index_prev,
+ q->matrix_coeff_index_now);
+
+channel_weighting(ou
Re: [FFmpeg-devel] [PATCH] tcp: set socket buffer sizes before listen/connect/accept
Ping! Anyone have a chance to look at this issue/patch?
Thanks,
Joel
> On Jan 9, 2017, at 2:54 PM, Joel Cunningham wrote:
>
> From e24d95c0e06a878d401ee34fd6742fcaddeeb95f Mon Sep 17 00:00:00 2001
> From: Joel Cunningham
> Date: Mon, 9 Jan 2017 13:37:51 -0600
> Subject: [PATCH] tcp: set socket buffer sizes before listen/connect/accept
>
> Attempting to set SO_RCVBUF and SO_SNDBUF on TCP sockets after connection
> establishment is incorrect and some stacks ignore the set call on the socket
> at
> this point. This has been observed on MacOS/iOS. Windows 7 has some peculiar
> behavior where setting SO_RCVBUF after applies only if the buffer is
> increasing
> from the default while decreases are ignored. This is possibly how the
> incorrect
> usage has gone unnoticed
>
> Unix Network Programming Vol. 1: The Sockets Networking API (3rd edition,
> seciton 7.5):
>
> "When setting the size of the TCP socket receive buffer, the ordering of the
> function calls is important. This is because of TCP's window scale option,
> which is exchanged with the peer on SYN segments when the connection is
> established. For a client, this means the SO_RCVBUF socket option must be
> set before calling connect. For a server, this means the socket option must
> be set for the listening socket before calling listen. Setting this option
> for the connected socket will have no effect whatsoever on the possible window
> scale option because accept does not return with the connected socket until
> TCP's three-way handshake is complete. This is why the option must be set on
> the listening socket. (The sizes of the socket buffers are always inherited
> from
> the listening socket by the newly created connected socket)"
>
> Signed-off-by: Joel Cunningham
> ---
> libavformat/tcp.c | 17 +
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/libavformat/tcp.c b/libavformat/tcp.c
> index 25abafc..5f00ba7 100644
> --- a/libavformat/tcp.c
> +++ b/libavformat/tcp.c
> @@ -140,6 +140,15 @@ static int tcp_open(URLContext *h, const char *uri, int
> flags)
> goto fail;
> }
>
> +/* Set the socket's send or receive buffer sizes, if specified.
> + If unspecified or setting fails, system default is used. */
> +if (s->recv_buffer_size > 0) {
> +setsockopt (fd, SOL_SOCKET, SO_RCVBUF, &s->recv_buffer_size, sizeof
> (s->recv_buffer_size));
> +}
> +if (s->send_buffer_size > 0) {
> +setsockopt (fd, SOL_SOCKET, SO_SNDBUF, &s->send_buffer_size, sizeof
> (s->send_buffer_size));
> +}
> +
> if (s->listen == 2) {
> // multi-client
> if ((ret = ff_listen(fd, cur_ai->ai_addr, cur_ai->ai_addrlen)) < 0)
> @@ -164,14 +173,6 @@ static int tcp_open(URLContext *h, const char *uri, int
> flags)
>
> h->is_streamed = 1;
> s->fd = fd;
> -/* Set the socket's send or receive buffer sizes, if specified.
> - If unspecified or setting fails, system default is used. */
> -if (s->recv_buffer_size > 0) {
> -setsockopt (fd, SOL_SOCKET, SO_RCVBUF, &s->recv_buffer_size, sizeof
> (s->recv_buffer_size));
> -}
> -if (s->send_buffer_size > 0) {
> -setsockopt (fd, SOL_SOCKET, SO_SNDBUF, &s->send_buffer_size, sizeof
> (s->send_buffer_size));
> -}
>
> freeaddrinfo(ai);
> return 0;
> --
> 2.10.0
> ___
> 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] HTTP: optimize forward seek performance
Ping! Anyone have a chance to review my latest patch which leverages the read-ahead/discard logic in avio_seek? Thanks, Joel > On Jan 13, 2017, at 11:36 AM, Joel Cunningham wrote: > >> >> On Jan 12, 2017, at 10:59 AM, Joel Cunningham wrote: >> >> Nicolas, >> >> I’ve found existing “read-ahead logic” in avio_seek to do what I’ve >> implemented in http_stream_forward(). This is controlled by >> SHORT_SEEK_THRESHOLD, currently set to 4KB. I proto-typed increasing this >> to the 256KB (matches the initial TCP window in my test setup) and saw the >> same number of reduction in HTTP GETs and the number of seeks! Thanks for >> the heads up, this should reduce my patch size! >> >> I could plumb this setting (s->short_seek_threshold) to a URL function that >> would get the desired value from HTTP/TCP. Looking at how >> ffio_init_context() is implemented, it doesn’t appear to have access to the >> URLContext pointer. Any guidance on how I can plumb the protocol layer with >> aviobuf without a public API change? >> >> Thanks, >> >> Joel > > > I managed to figure the plumbing out in a way that doesn’t modify any public > APIs. I added a new callback to struct AVIOContext that gets a short seek > threshold value from URLContext if available (otherwise fallback to > s->short_seek_threshold). This allows using the read-ahead/discard logic in > avio_seek and eliminates my forwarding logic in the HTTP layer (thanks to > Nicolas for pointing me in this direction). See new patch below > > I also updated my commit message to include assumptions/considerations about > congestion control on the sender (thanks to Andy for calling out the > discussion on it). Lastly, I have upload new captures/log in dropbox for > those that want to take a look at my testing output (see > ffplay_short_seek_output.log and mac-ffplay-short-seek-patch.pcapng) > > https://www.dropbox.com/sh/4q4ru8isdv22joj/AABU3XyXmgLMiEFqucf1LdZ3a?dl=0 > > Thanks for the feedback so far, > > Joel > > From 9bb2f184591c2d6e6a91d3760e63b013ca4c95e5 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 inital 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 the connection, > then open a new one within the same window's worth of data, we discard > from the current offset till the end of the window. Then on the new > connection the server ends up re-sending the previous data from new > offset till the end of old window. > > Example (assumes full window utilization): > > TCP window size: 64KB > Position: 32KB > Forward seek position: 40KB > > * (Next window) > 32KB |--| 96KB |---| 160KB >* > 40KB |---| 104KB > > Re-sent amount: 96KB - 40KB = 56KB > > For a real world test example, I have MP4 file of ~25MB, which ffplay > only reads ~16MB and performs 177 seeks. With current ffmpeg, this results > in 177 HTTP GETs and ~73MB worth of TCP data communication. With this > patch, ffmpeg issues 4 HTTP GETs and 3 seeks for a total of ~22MB of TCP data > communication. > > To support this feature, the short seek logic in avio_seek() has been > extended to call a function to get the short seek threshold value. This > callback has been plumbed to the URLProtocol structure, which now has > infrastructure in HTTP and TCP to get the underlying receiver window size > via SO_RCVBUF. If the underlying URL and protocol don't support returning > a short seek threshold, the default s->short_seek_threshold is used > > This feature has been tested on Windows 7 and MacOS/iOS. Windows support > is slightly complicated by the fact that when TCP window auto-tuning is > enabled, SO_RCVBUF doesn't report the real window size, but it doe
[FFmpeg-devel] [PATCH] avformat/Makefile: fix compilation of testprogs when networking is disabled
Signed-off-by: Tobias Rapp --- libavformat/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/Makefile b/libavformat/Makefile index f30bc94..8aab4d1 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -595,7 +595,8 @@ TESTPROGS = seek \ url \ # async \ -TESTPROGS-$(CONFIG_FIFO_MUXER) += fifo_muxer +FIFO-MUXER-TESTPROGS-$(CONFIG_NETWORK) += fifo_muxer +TESTPROGS-$(CONFIG_FIFO_MUXER) += $(FIFO-MUXER-TESTPROGS-yes) TESTPROGS-$(CONFIG_FFRTMPCRYPT_PROTOCOL) += rtmpdh TESTPROGS-$(CONFIG_MOV_MUXER)+= movenc TESTPROGS-$(CONFIG_NETWORK) += noproxy -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/mov: Export vendor metadata
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(&st->metadata, "vendor", vendor, 0);
Does this mean transcoding to a format with per-stream tags will add
this as a tag?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] FFmpeg 3.3
2017.01.25. 9:40 keltezéssel, Tobias Rapp írta: On 23.01.2017 02:11, Michael Niedermayer wrote: Its a while since the last relase so ill make 3.3 within the next week(s) Name suggestions needed like always What about "Hilbert", named after David Hilbert? +1 vote I also intend to make some new point releases from the currently maintained branches if someone wants to backport something If not already done, please backport commit http://git.videolan.org/?p=ffmpeg.git;a=commit;h=6d579d7c1bdc4126955cae7f385208e455685986 Regards, Tobias ___ 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] GSoC 2017
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] [PATCH] avcodec/utils: Fix memleak with subtitles and sidedata
Fixes: 454/fuzz-3-ffmpeg_SUBTITLE_AV_CODEC_ID_MOV_TEXT_fuzzer
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/utils.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 4ae752ff2f..f8ec4c1ae6 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2714,13 +2714,19 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx,
AVSubtitle *sub,
avctx->pkt_timebase, ms);
}
+if (avctx->codec_descriptor->props & AV_CODEC_PROP_BITMAP_SUB)
+sub->format = 0;
+else if (avctx->codec_descriptor->props & AV_CODEC_PROP_TEXT_SUB)
+sub->format = 1;
+
for (i = 0; i < sub->num_rects; i++) {
if (sub->rects[i]->ass && !utf8_check(sub->rects[i]->ass)) {
av_log(avctx, AV_LOG_ERROR,
"Invalid UTF-8 in decoded subtitles text; "
"maybe missing -sub_charenc option\n");
avsubtitle_free(sub);
-return AVERROR_INVALIDDATA;
+ret = AVERROR_INVALIDDATA;
+break;
}
}
@@ -2731,10 +2737,6 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx,
AVSubtitle *sub,
av_packet_unref(&pkt_recoded);
}
-if (avctx->codec_descriptor->props & AV_CODEC_PROP_BITMAP_SUB)
-sub->format = 0;
-else if (avctx->codec_descriptor->props & AV_CODEC_PROP_TEXT_SUB)
-sub->format = 1;
avctx->internal->pkt = NULL;
}
--
2.11.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] FFmpeg 3.3
2017-01-25 12:09 GMT+01:00 Tobias Rapp : > Not sure about your definition of "regression". An issue that was not reproducible with earlier versions (of FFmpeg). > The given scenario works for FFmpeg <= 2.6 but fails with > versions between 2.7 and 3.2. Then the patch should of course be backported. > Besides, I'd think the fix is low on side effect risks. What are your > concerns? That people start backporting random patches (as happened in the past), making our releases that unfortunately see very limited support even worse (backporting patches of course leads inevitably to even more regressions). Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] FFmpeg 3.3
On 25.01.2017 11:59, Carl Eugen Hoyos wrote: 2017-01-25 11:47 GMT+01:00 Tobias Rapp : On 25.01.2017 11:23, Carl Eugen Hoyos wrote: 2017-01-25 9:40 GMT+01:00 Tobias Rapp : If not already done, please backport commit http://git.videolan.org/?p=ffmpeg.git;a=commit;h=6d579d7c This doesn't look as if it fixes a regression, does it? It fixes the scenario where you create an AVI file > 256GiB with FFmpeg and then try to read it back. Versions after commit bbcc09518e0d1efc189a43ff0120c1a31f51c802 and without the fix will produce a longer video stream. So it does not fix a regression? In this case, I suggest not to backport the patch. Not sure about your definition of "regression". The given scenario works for FFmpeg <= 2.6 but fails with versions between 2.7 and 3.2. Besides, I'd think the fix is low on side effect risks. What are your concerns? Regards, Tobias ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] FFmpeg 3.3
2017-01-25 11:47 GMT+01:00 Tobias Rapp : > On 25.01.2017 11:23, Carl Eugen Hoyos wrote: >> >> 2017-01-25 9:40 GMT+01:00 Tobias Rapp : >> >>> If not already done, please backport commit >>> http://git.videolan.org/?p=ffmpeg.git;a=commit;h=6d579d7c >> >> This doesn't look as if it fixes a regression, does it? > > It fixes the scenario where you create an AVI file > 256GiB > with FFmpeg and then try to read it back. Versions after > commit bbcc09518e0d1efc189a43ff0120c1a31f51c802 > and without the fix will produce a longer video stream. So it does not fix a regression? In this case, I suggest not to backport the patch. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] FFmpeg 3.3
On 25.01.2017 11:23, Carl Eugen Hoyos wrote: 2017-01-25 9:40 GMT+01:00 Tobias Rapp : If not already done, please backport commit http://git.videolan.org/?p=ffmpeg.git;a=commit;h=6d579d7c This doesn't look as if it fixes a regression, does it? It fixes the scenario where you create an AVI file > 256GiB with FFmpeg and then try to read it back. Versions after commit bbcc09518e0d1efc189a43ff0120c1a31f51c802 and without the fix will produce a longer video stream. Regards, Tobias ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] FFmpeg 3.3
2017-01-25 9:40 GMT+01:00 Tobias Rapp : > If not already done, please backport commit > http://git.videolan.org/?p=ffmpeg.git;a=commit;h=6d579d7c This doesn't look as if it fixes a regression, does it? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] FFmpeg 3.3
On 23.01.2017 02:11, Michael Niedermayer wrote: Its a while since the last relase so ill make 3.3 within the next week(s) Name suggestions needed like always What about "Hilbert", named after David Hilbert? I also intend to make some new point releases from the currently maintained branches if someone wants to backport something If not already done, please backport commit http://git.videolan.org/?p=ffmpeg.git;a=commit;h=6d579d7c1bdc4126955cae7f385208e455685986 Regards, Tobias ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
