[FFmpeg-devel] [PATCH v8] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

Forgot to remove an av_free(). Patch explanation follows:

Palettized QuickTime video in Matroska has hitherto not been recognized
whatsoever, and the "palette" used has been completely random.

The patch for matroskadec.c fixes this issue by adding a palette side
data packet in matroska_deliver_packet(), much in the same way as it's
done in mov.c.

The change to mov.c consists mainly of moving the palette handling from
the mov_parse_stsd_video() function to a new ff_get_qtpalette() function
in the new file qtpalette.c, which is shared by both matroskadec.c and
mov.c.

Since the ff_get_qtpalette() function has to parse the video sample
description in two different ways, i.e. 1) by using the in-memory
private data when called from matroskadec.c, and 2) by reading from the
file when called from mov.c, it has to use a separate variable for each
of these sources. It may seem slightly redundant, but it is
unfortunately necessary. I would prefer that nobody touches what I've
been doing, since it's working fine right now, and it's very easy to
break things if you try to "improve it". Believe me, I've been there.

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
>From c872c430d93cc450255306f52256fcbf808c3248 Mon Sep 17 00:00:00 2001
From: Mats Peterson 
Date: Sun, 27 Dec 2015 11:39:21 +0100
Subject: [PATCH v8] lavf: palettized QuickTime video in Matroska

Forgot to remove an av_free(). Patch explanation follows:

Palettized QuickTime video in Matroska has hitherto not been recognized
whatsoever, and the "palette" used has been completely random.

The patch for matroskadec.c fixes this issue by adding a palette side
data packet in matroska_deliver_packet(), much in the same way as it's
done in mov.c.

The change to mov.c consists mainly of moving the palette handling from
the mov_parse_stsd_video() function to a new ff_get_qtpalette() function
in the new file qtpalette.c, which is shared by both matroskadec.c and
mov.c.

Since the ff_get_qtpalette() function has to parse the video sample
description in two different ways, i.e. 1) by using the in-memory
private data when called from matroskadec.c, and 2) by reading from the
file when called from mov.c, it has to use a separate variable for each
of these sources. It may seem slightly redundant, but it is
unfortunately necessary. I would prefer that nobody touches what I've
been doing, since it's working fine right now, and it's very easy to
break things if you try to "improve it". Believe me, I've been there.

In matroskadec.c, I'm also putting the palette in 'extradata', like it's
done for V_MS/VFW/FOURCC; this is a requirement in order for MPlayer to
use the correct palette. And why is that, you may wonder. Well, it's
because for some mysterious reason, MPlayer adds *another* palette side
data packet *after* the one added in matroskadec.c. It uses whatever is
in extradata as the palette when adding this packet.

Video samples for testing are available at
https://drive.google.com/open?id=0B3_pEBoLs0faWElmM2FnLTZYNlk.

---
 libavformat/Makefile  |1 +
 libavformat/matroskadec.c |   30 ++-
 libavformat/mov.c |   80 
 libavformat/qtpalette.c   |  128 +
 libavformat/qtpalette.h   |6 ++-
 5 files changed, 173 insertions(+), 72 deletions(-)
 create mode 100644 libavformat/qtpalette.c

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 110e9e3..e03c73e 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -18,6 +18,7 @@ OBJS = allformats.o \
mux.o\
options.o\
os_support.o \
+   qtpalette.o  \
riff.o   \
sdp.o\
url.o\
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index c574749..cc3cdd0 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -64,6 +64,8 @@
 #include 
 #endif
 
+#include "qtpalette.h"
+
 typedef enum {
 EBML_NONE,
 EBML_UINT,
@@ -312,6 +314,9 @@ typedef struct MatroskaDemuxContext {
 
 /* WebM DASH Manifest live flag/ */
 int is_live;
+
+uint32_t palette[AVPALETTE_COUNT];
+int has_palette;
 } MatroskaDemuxContext;
 
 typedef struct MatroskaBlock {
@@ -1856,7 +1861,7 @@ static int matroska_parse_tracks(AVFormatContext *s)
 fourcc   = st->codec->codec_tag;
 extradata_offset = FFMIN(track->codec_priv.size, 18);
 } else if (!strcmp(track->codec_id, "A_QUICKTIME")
-   && (track->codec_priv.size >= 86)
+   && (track->codec_priv.size >= 36)
&& (track->codec_priv.data)) {
 fourcc = AV_RL32(track->codec_priv.data + 4);
 codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
@@ -1881,6 +1886,20 @@ static int matroska_parse_tracks(AVFormatContext *s)
 av_log(matroska->ctx, AV_LOG_ERROR,
 

[FFmpeg-devel] [PATCH] avcodec/on2avc: Fix stability issues with scale_tab generation

[Michael Niedermayer]

From: Michael Niedermayer 

This also simplifies the code
the resulting values are binary identical to what pow(10, i/10.0) produces
---
 libavcodec/on2avc.c |   37 -
 1 file changed, 4 insertions(+), 33 deletions(-)

diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
index 3309d99..e6ccd88 100644
--- a/libavcodec/on2avc.c
+++ b/libavcodec/on2avc.c
@@ -912,23 +912,7 @@ static av_cold void on2avc_free_vlcs(On2AVCContext *c)
 static av_cold int on2avc_decode_init(AVCodecContext *avctx)
 {
 On2AVCContext *c = avctx->priv_data;
-int i, ph;
-/* 10^(i*0.1) for 0 <= i < 10 */
-/* TODO: possibly statically allocate scale_tab; this may help with FATE
- * and reproducibility if the binary size is not impacted much */
-static const double exp10_lut[] = {
-1,
-1.2589254117941673,
-1.5848931924611136,
-1.9952623149688795,
-2.5118864315095806,
-3.1622776601683795,
-3.9810717055349727,
-5.0118723362727229,
-6.3095734448019334,
-7.9432823472428158,
-};
-int64_t exp10_base = 10;
+int i;
 
 if (avctx->channels > 2U) {
 avpriv_request_sample(avctx, "Decoding more than 2 channels");
@@ -950,23 +934,10 @@ static av_cold int on2avc_decode_init(AVCodecContext 
*avctx)
 av_log(avctx, AV_LOG_WARNING,
"Stereo mode support is not good, patch is welcome\n");
 
-
-/* Fast and more accurate way of doing for (i = 0; i < 20; i++)
-c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 16) / 32;
+for (i = 0; i < 20; i++)
+c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 16 - 0.01) / 32;
 for (; i < 128; i++)
-c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 0.5); */
-for (i = 0; i < 10; i++) {
-c->scale_tab[i] = ceil(exp10_lut[i] * 16) / 32;
-c->scale_tab[i+10] = ceil(exp10_lut[i] * 160) / 32;
-}
-
-for (i = 20, ph = 0; i < 128; i++, ph++) {
-if (i % 10 == 0) {
-exp10_base *= 10;
-ph = 0;
-}
-c->scale_tab[i] = ceil(exp10_base * exp10_lut[ph] * 0.5);
-}
+c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 0.5 - 0.01);
 
 if (avctx->sample_rate < 32000 || avctx->channels == 1)
 memcpy(c->long_win, ff_on2avc_window_long_24000,
-- 
1.7.9.5

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

Re: [FFmpeg-devel] [PATCH 11/15] lavc/on2avc: replace pow(10, x) by exp10(x)

[Michael Niedermayer]

On Sat, Dec 26, 2015 at 06:37:31PM -0800, Ganesh Ajjanagadde wrote:
> On Sat, Dec 26, 2015 at 6:19 PM, Michael Niedermayer
>  wrote:
> > On Sat, Dec 26, 2015 at 05:23:55PM -0800, Ganesh Ajjanagadde wrote:
> >> On Sat, Dec 26, 2015 at 4:20 PM, Ganesh Ajjanagadde  
> >> wrote:
> >> > On Sat, Dec 26, 2015 at 4:08 PM, James Almer  wrote:
> >> >> On 12/23/2015 3:47 PM, Ganesh Ajjanagadde wrote:
> >> >>> exp10, introduced recently, is superior for the purpose.
> >> >>>
> >> >>> Signed-off-by: Ganesh Ajjanagadde 
> >> >>> ---
> >> >>>  libavcodec/on2avc.c | 5 +++--
> >> >>>  1 file changed, 3 insertions(+), 2 deletions(-)
> >> >>>
> >> >>> diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
> >> >>> index 04c8e41..0409b3e 100644
> >> >>> --- a/libavcodec/on2avc.c
> >> >>> +++ b/libavcodec/on2avc.c
> >> >>> @@ -22,6 +22,7 @@
> >> >>>
> >> >>>  #include "libavutil/channel_layout.h"
> >> >>>  #include "libavutil/float_dsp.h"
> >> >>> +#include "libavutil/libm.h"
> >> >>>  #include "avcodec.h"
> >> >>>  #include "bytestream.h"
> >> >>>  #include "fft.h"
> >> >>> @@ -934,9 +935,9 @@ static av_cold int 
> >> >>> on2avc_decode_init(AVCodecContext *avctx)
> >> >>> "Stereo mode support is not good, patch is welcome\n");
> >> >>>
> >> >>>  for (i = 0; i < 20; i++)
> >> >>> -c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 16) / 32;
> >> >>> +c->scale_tab[i] = ceil(exp10(i * 0.1) * 16) / 32;
> >> >>>  for (; i < 128; i++)
> >> >>> -c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 0.5);
> >> >>> +c->scale_tab[i] = ceil(exp10(i * 0.1) * 0.5);
> >> >>>
> >> >>>  if (avctx->sample_rate < 32000 || avctx->channels == 1)
> >> >>>  memcpy(c->long_win, ff_on2avc_window_long_24000,
> >> >>
> >> >> This apparently broke ICC
> >> >>
> >> >> http://fate.ffmpeg.org/report.cgi?time=20151226215846=x86_64-linux-gnu-icc-2011.4.191
> >> >> http://fate.ffmpeg.org/report.cgi?time=20151226235348=x86_64-linux-gnu-icc-2011_sp1.13.367
> >> >> http://fate.ffmpeg.org/report.cgi?time=20151226203729=x86_64-archlinux-icc-2013
> >> >
> >> > Thanks for the report. A couple of questions:
> >> > 1. Is there an easy way to acquire icc so that I can reproduce?
> >> > GCC/Clang are perfectly fine with it.
> >> > 2. Do you know what the ICC platforms use for exp2?
> >> >
> >> > In the absence of remarks and my own inability to fix this, I will
> >> > revert tonight.
> >> > BTW, please let me know the general policy for this kind of breakage:
> >> > i.e, how quickly do such regressions need to be fixed.
> >>
> >> Different fix pushed that also speeds up things.
> >
> > but speed doesnt really matter for this code while maintainability
> > IMHO matters more
> 
> Definitely, I did not do it for speed actually, speed was a side
> effect - this is also why the first line of the message does not
> mention speed effect. I did this because it improved accuracy on
> clang/gcc (as also mentioned in the commit message). In any case, I
> was looking for a quick fix since I assumed ICC regression is serious,
> and wanted to do so in a way that improves numerical accuracy.
> 
> >
> > either way, the code is numerically unstable as some of the arguments
> > to ceil() fall exactly on the mid point between different outputs
> > this is still so after the change and would be in case of a revert too
> 
> Yes. I think something needs to be done to FATE to fix this, but I have no 
> idea.

patch posted

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- 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 v8] lavf: palettized QuickTime video in Matroska

[Hendrik Leppkes]

On Sun, Dec 27, 2015 at 11:44 AM, Mats Peterson
 wrote:
>
> Since the ff_get_qtpalette() function has to parse the video sample
> description in two different ways, i.e. 1) by using the in-memory
> private data when called from matroskadec.c, and 2) by reading from the
> file when called from mov.c, it has to use a separate variable for each
> of these sources. It may seem slightly redundant, but it is
> unfortunately necessary. I would prefer that nobody touches what I've
> been doing, since it's working fine right now, and it's very easy to
> break things if you try to "improve it". Believe me, I've been there.
>

Just to make it perfectly clear that there are no misunderstandings:
A shared utility function should offer one clear calling interface,
not two just because its convenient. Anything else is a maintenance
nightmare.

Patch not acceptable unless this is corrected.

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

Re: [FFmpeg-devel] [PATCH v8] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/27/2015 12:10 PM, Hendrik Leppkes wrote:

On Sun, Dec 27, 2015 at 11:44 AM, Mats Peterson
 wrote:


Since the ff_get_qtpalette() function has to parse the video sample
description in two different ways, i.e. 1) by using the in-memory
private data when called from matroskadec.c, and 2) by reading from the
file when called from mov.c, it has to use a separate variable for each
of these sources. It may seem slightly redundant, but it is
unfortunately necessary. I would prefer that nobody touches what I've
been doing, since it's working fine right now, and it's very easy to
break things if you try to "improve it". Believe me, I've been there.



Just to make it perfectly clear that there are no misunderstandings:
A shared utility function should offer one clear calling interface,
not two just because its convenient. Anything else is a maintenance
nightmare.

Patch not acceptable unless this is corrected.

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



You're not the maintainer, once again.

Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v6] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/27/2015 10:30 AM, Hendrik Leppkes wrote:

On Sun, Dec 27, 2015 at 4:42 AM, Mats Peterson
 wrote:

On 12/27/2015 03:57 AM, Mats Peterson wrote:


On 12/27/2015 03:03 AM, Michael Niedermayer wrote:


+
+if (!(stsd = av_malloc(70)))
+return AVERROR(ENOMEM);



the malloc is unneeded, an array on the stack could be used (its just
a fixed 70 bytes)
this would also simplify the error handling



Yes, I thought so. I tried to be "a good boy", but that was obviously to
no avail ;)


+int ff_get_qtpalette(int codec_id, uint8_t *stsd, AVIOContext *pb,
+uint32_t *palette);
+



missing doxy documentation, missing "const" for unchanged arrays
also why does this need a "byte" array and a AVIOContext as input
arguments ?
iam asking as this looks a bit confusing with 2 inputs ...



Regarding doxy documentation, I notice several files in libavformat are
lacking doxy documentation (if what you mean by "doxy documentation" is
a comment beginning with /**). I don't know what to put it in either, at
that. Please help me out.

And regarding two inputs, well, the problem is that matroskadec.c has
the video sample description stored in its in-memory private data, while
mov.c reads the video sample description from the file. I don't want to
mess too much with the logic in mov.c, that's why I provide both a
"memory" and a "file" input. Confusing, yes, slightly, but necessary as
long as you want a common function to be called from both sources. If
anyone else manages to come up with something better WITHOUT BREAKING
IT, no problem. It does take some knowledge about the structure of a
QuickTime video sample description.

Mats

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



Actually I would prefer that nobody touches what I've been doing, since it
works just fine right now, and it can be easily broken if you start trying
to "improve it". Belive me, I've tried.



And we would prefer if code is actually clean and not a convoluted
mess, and if you don't want us improving it, and you don't want to do
it yourself, then thats too bad.

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



It's not a convoluted mess. It's better to have one function that is 
called by two different demuxers than duplicating code. And since it 
works well right now, why change it? We will see what Michael 
Niedermayer says about this. After all, he is the maintainer.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v6] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/27/2015 10:38 AM, Mats Peterson wrote:

On 12/27/2015 10:30 AM, Hendrik Leppkes wrote:

On Sun, Dec 27, 2015 at 4:42 AM, Mats Peterson
 wrote:

On 12/27/2015 03:57 AM, Mats Peterson wrote:


On 12/27/2015 03:03 AM, Michael Niedermayer wrote:


+
+if (!(stsd = av_malloc(70)))
+return AVERROR(ENOMEM);



the malloc is unneeded, an array on the stack could be used (its just
a fixed 70 bytes)
this would also simplify the error handling



Yes, I thought so. I tried to be "a good boy", but that was
obviously to
no avail ;)


+int ff_get_qtpalette(int codec_id, uint8_t *stsd, AVIOContext *pb,
+uint32_t *palette);
+



missing doxy documentation, missing "const" for unchanged arrays
also why does this need a "byte" array and a AVIOContext as input
arguments ?
iam asking as this looks a bit confusing with 2 inputs ...



Regarding doxy documentation, I notice several files in libavformat are
lacking doxy documentation (if what you mean by "doxy documentation" is
a comment beginning with /**). I don't know what to put it in
either, at
that. Please help me out.

And regarding two inputs, well, the problem is that matroskadec.c has
the video sample description stored in its in-memory private data,
while
mov.c reads the video sample description from the file. I don't want to
mess too much with the logic in mov.c, that's why I provide both a
"memory" and a "file" input. Confusing, yes, slightly, but necessary as
long as you want a common function to be called from both sources. If
anyone else manages to come up with something better WITHOUT BREAKING
IT, no problem. It does take some knowledge about the structure of a
QuickTime video sample description.

Mats

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



Actually I would prefer that nobody touches what I've been doing,
since it
works just fine right now, and it can be easily broken if you start
trying
to "improve it". Belive me, I've tried.



And we would prefer if code is actually clean and not a convoluted
mess, and if you don't want us improving it, and you don't want to do
it yourself, then thats too bad.

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



It's not a convoluted mess. It's better to have one function that is
called by two different demuxers than duplicating code. And since it
works well right now, why change it? We will see what Michael
Niedermayer says about this. After all, he is the maintainer.

Mats



Read the explanation above thoroughly, and you will hopefully understand 
the problem.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavu/libm: change macros to functions

[Ronald S. Bultje]

Hi,

On Thu, Dec 24, 2015 at 1:32 PM, Ganesh Ajjanagadde 
wrote:

> In the standard library, these are functions. We should match it; there
> is no reason for these to be macros.
>
> While at it, add some trivial comments for readability and correct an
> incorrect (at standard double precision) M_LN2 constant used in the exp2
> fallback.


For bisect purposes, the M_LN2 change should be a separate patch IMO. I
don't have objections to that one.

As for the rest, I'm not against it, but this stuff is extremely brittle so
please test it extremely carefully on various configs.

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

Re: [FFmpeg-devel] [PATCH] avcodec/on2avc: Fix stability issues with scale_tab generation

[Michael Niedermayer]

On Sun, Dec 27, 2015 at 07:40:53AM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Sun, Dec 27, 2015 at 6:02 AM, Michael Niedermayer 
> wrote:
> 
> > +for (i = 0; i < 20; i++)
> > +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 16 - 0.01) / 32;
> >  for (; i < 128; i++)
> > +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 0.5 - 0.01);
> >
> 
> To innocent bystanders, this is hard to understand. Let's keep it a habit
> to document things (what and why), where the "what" portion is probably
> "this code emulates pow(10, x) with ff_pow10(x - 0.01)". As for why (most
> specifically, why the 0.01?), I'm going to assume that here, you're trying
> to get unit integers to not go to unit.000[..]001, so you subtract 0.01
> before the ceil so it works fine again to get the exact unit integer output
> number. If that's correct, please add a comment saying that, and then lgtm.

yes
changed
applied
thx


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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras


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

Re: [FFmpeg-devel] [PATCH] lavu/libm: change macros to functions

[Ganesh Ajjanagadde]

On Sun, Dec 27, 2015 at 5:38 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Thu, Dec 24, 2015 at 1:32 PM, Ganesh Ajjanagadde 
> wrote:
>>
>> In the standard library, these are functions. We should match it; there
>> is no reason for these to be macros.
>>
>> While at it, add some trivial comments for readability and correct an
>> incorrect (at standard double precision) M_LN2 constant used in the exp2
>> fallback.
>
>
> For bisect purposes, the M_LN2 change should be a separate patch IMO. I
> don't have objections to that one.

The ln2 change went in already:
5630ed5be64fef3fd70cb93a7623d46afa0c83e6 with some very minor stuff
(comment additions),

>
> As for the rest, I'm not against it, but this stuff is extremely brittle so
> please test it extremely carefully on various configs.

Yes, I have dropped the rest on my end since:
1. I lack an array of configs (only GNU/Linux+clang/gcc, Mac OS X with
difficulty).
2. I am not knowledgable on this aspect.
3. It should be done slowly IMHO to minimize scope of regressions. For
a start, maybe focusing on the float variants (expf, etc) may be
useful.

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

Re: [FFmpeg-devel] [PATCH] avformat: stop exporting ffurl_read_complete, ffurl_seek and ffurl_size

[Hendrik Leppkes]

On Sun, Dec 27, 2015 at 7:32 PM, Jan Ehrhardt  wrote:
> Andreas Cadhalpun in gmane.comp.video.ffmpeg.devel (Tue, 27 Oct 2015
> 22:52:27 +0100):
>>> On Tue, Oct 27, 2015 at 9:58 PM, Andreas Cadhalpun
 diff --git a/libavformat/libavformat.v b/libavformat/libavformat.v
 index e90aef7..a00a309 100644
 --- a/libavformat/libavformat.v
 +++ b/libavformat/libavformat.v
 @@ -10,9 +10,6 @@ LIBAVFORMAT_$MAJOR {
 -ffurl_read_complete;
 -ffurl_seek;
 -ffurl_size;
>>
>>OK. I'll wait a bit to see if someone else wants to comment on this patch.
>
> There is a PHP extension that uses ffurl_seek and ffurl_read_complete:
> https://github.com/chung-leong/av/blob/master/faststart.c#L76
>
> These functions are also documented at
> http://www.ffmpeg.org/doxygen/trunk/avio_8c.html
>
> How should an external developer know what functions are publically
> available?

The doxygen you referenced documents everything, including internal
data structures.
It may be a bit unfortunate choice to have that on the website, but
its probably also used by developers of FFmpeg.

If you want to see public functions, best would be to check the
installed headers. avio.h for example doesn't export any non-public
functions (while of course avio.c contains them, which is what you
linked the doxygen of)

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

Re: [FFmpeg-devel] [PATCH] avcodec/utils: fix check for invalid timebase when decoding subtitles

[Andreas Cadhalpun]

On 27.12.2015 20:10, Hendrik Leppkes wrote:
> Invalid timebases have a zero numerator, not denominator.

A timebase with zero numerator is probably invalid, but a timebase
with zero denominator is not even well defined.
So this comment doesn't seem quite right.

> Fixes a integer divison by zero.
> ---
>  libavcodec/utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 19f3f0a..33295ed 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2435,7 +2435,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
> AVSubtitle *sub,
>  } else {
>  avctx->internal->pkt = _recoded;
>  
> -if (avctx->pkt_timebase.den && avpkt->pts != AV_NOPTS_VALUE)
> +if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE)
>  sub->pts = av_rescale_q(avpkt->pts,
>  avctx->pkt_timebase, AV_TIME_BASE_Q);
>  ret = avctx->codec->decode(avctx, sub, got_sub_ptr, 
> _recoded);
> 

If avctx->pkt_timebase.den is 0, calling av_rescale_q here will trigger the
av_assert2(c > 0) in av_rescale_rnd, so removing this check isn't good.

Why not check for both num and den?

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

Re: [FFmpeg-devel] [PATCH] avcodec/utils: fix check for invalid timebase when decoding subtitles

[Andreas Cadhalpun]

On 27.12.2015 20:43, Hendrik Leppkes wrote:
> On Sun, Dec 27, 2015 at 8:29 PM, Andreas Cadhalpun
>  wrote:
>> On 27.12.2015 20:10, Hendrik Leppkes wrote:
>>> Invalid timebases have a zero numerator, not denominator.
>>
>> A timebase with zero numerator is probably invalid, but a timebase
>> with zero denominator is not even well defined.
>> So this comment doesn't seem quite right.
>>
>>> Fixes a integer divison by zero.
>>> ---
>>>  libavcodec/utils.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>>> index 19f3f0a..33295ed 100644
>>> --- a/libavcodec/utils.c
>>> +++ b/libavcodec/utils.c
>>> @@ -2435,7 +2435,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
>>> AVSubtitle *sub,
>>>  } else {
>>>  avctx->internal->pkt = _recoded;
>>>
>>> -if (avctx->pkt_timebase.den && avpkt->pts != AV_NOPTS_VALUE)
>>> +if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE)
>>>  sub->pts = av_rescale_q(avpkt->pts,
>>>  avctx->pkt_timebase, 
>>> AV_TIME_BASE_Q);
>>>  ret = avctx->codec->decode(avctx, sub, got_sub_ptr, 
>>> _recoded);
>>>
>>
>> If avctx->pkt_timebase.den is 0, calling av_rescale_q here will trigger the
>> av_assert2(c > 0) in av_rescale_rnd, so removing this check isn't good.
>>
>> Why not check for both num and den?
>>
> 
> We never check both, invalid timebase is {0, 1}, anything else needs
> to have values in both.

I'd call this unknown timebase, as {0, 1} is the initialization value.
A {1, 0} timebase is certainly not valid.

> All timebase checks are for num only.

The check here was for den and there is a similar check in 
teletext_decode_frame.

> Its an assert2 for a reason (ie. a developer tool), its checked in an
> error condition right after and returns INT64_MIN.

It's an assert, because it should never happen.

If the check for den here was intended to prevent triggering such an assert,
then it would still be needed.
If on the other hand this check was meant to check for an uninitialized
pkt_timebase, then removing it would be fine.

So you can remove this check, if you're sure it's the second case.
But please clarify this in the commit message.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH] avformat: stop exporting ffurl_read_complete, ffurl_seek and ffurl_size

[Jan Ehrhardt]

Andreas Cadhalpun in gmane.comp.video.ffmpeg.devel (Tue, 27 Oct 2015
22:52:27 +0100):
>> On Tue, Oct 27, 2015 at 9:58 PM, Andreas Cadhalpun
>>> diff --git a/libavformat/libavformat.v b/libavformat/libavformat.v
>>> index e90aef7..a00a309 100644
>>> --- a/libavformat/libavformat.v
>>> +++ b/libavformat/libavformat.v
>>> @@ -10,9 +10,6 @@ LIBAVFORMAT_$MAJOR {
>>> -ffurl_read_complete;
>>> -ffurl_seek;
>>> -ffurl_size;
>
>OK. I'll wait a bit to see if someone else wants to comment on this patch.

There is a PHP extension that uses ffurl_seek and ffurl_read_complete:
https://github.com/chung-leong/av/blob/master/faststart.c#L76

These functions are also documented at
http://www.ffmpeg.org/doxygen/trunk/avio_8c.html

How should an external developer know what functions are publically
available?
-- 
Jan

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

Re: [FFmpeg-devel] [PATCH] avcodec/utils: fix check for invalid timebase when decoding subtitles

[Hendrik Leppkes]

On Sun, Dec 27, 2015 at 8:29 PM, Andreas Cadhalpun
 wrote:
> On 27.12.2015 20:10, Hendrik Leppkes wrote:
>> Invalid timebases have a zero numerator, not denominator.
>
> A timebase with zero numerator is probably invalid, but a timebase
> with zero denominator is not even well defined.
> So this comment doesn't seem quite right.
>
>> Fixes a integer divison by zero.
>> ---
>>  libavcodec/utils.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 19f3f0a..33295ed 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -2435,7 +2435,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
>> AVSubtitle *sub,
>>  } else {
>>  avctx->internal->pkt = _recoded;
>>
>> -if (avctx->pkt_timebase.den && avpkt->pts != AV_NOPTS_VALUE)
>> +if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE)
>>  sub->pts = av_rescale_q(avpkt->pts,
>>  avctx->pkt_timebase, 
>> AV_TIME_BASE_Q);
>>  ret = avctx->codec->decode(avctx, sub, got_sub_ptr, 
>> _recoded);
>>
>
> If avctx->pkt_timebase.den is 0, calling av_rescale_q here will trigger the
> av_assert2(c > 0) in av_rescale_rnd, so removing this check isn't good.
>
> Why not check for both num and den?
>

We never check both, invalid timebase is {0, 1}, anything else needs
to have values in both. All timebase checks are for num only.
Its an assert2 for a reason (ie. a developer tool), its checked in an
error condition right after and returns INT64_MIN.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

Removed the 'stsd' variable from ff_get_qtpalette() in qtpalette.c.
Updated the doxy documentation for ff_get_qtpalette() accordingly.

Description of patch follows:

Palettized QuickTime video in Matroska has hitherto not been recognized
whatsoever, and the "palette" used has been completely random.

The patch for matroskadec.c fixes this issue by adding a palette side
data packet in matroska_deliver_packet(), much in the same way as it's
done in mov.c.

The change to mov.c consists mainly of moving the palette handling from
the mov_parse_stsd_video() function to a new ff_get_qtpalette() function
in the new file qtpalette.c, which is shared by both matroskadec.c and
mov.c.

In matroskadec.c, I'm also putting the palette in 'extradata', like it's
done for V_MS/VFW/FOURCC; this is a requirement in order for MPlayer to
use the correct palette. And why is that, you may wonder. Well, it's
because for some mysterious reason, MPlayer adds *another* palette side
data packet *after* the one added in matroskadec.c. It uses whatever is
in extradata as the palette when adding this packet.

Video samples for testing are available at
https://drive.google.com/open?id=0B3_pEBoLs0faWElmM2FnLTZYNlk.

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
>From a15ec091ba494a5e5a8f710bdb97e869c9f702e7 Mon Sep 17 00:00:00 2001
From: Mats Peterson 
Date: Sun, 27 Dec 2015 21:28:09 +0100
Subject: [PATCH v9] lavf: palettized QuickTime video in Matroska

Removed the 'stsd' variable from ff_get_qtpalette() in qtpalette.c.
Updated the doxy documentation for ff_get_qtpalette() accordingly.

Original description of patch follows:

Palettized QuickTime video in Matroska has hitherto not been recognized
whatsoever, and the "palette" used has been completely random.

The patch for matroskadec.c fixes this issue by adding a palette side
data packet in matroska_deliver_packet(), much in the same way as it's
done in mov.c.

The change to mov.c consists mainly of moving the palette handling from
the mov_parse_stsd_video() function to a new ff_get_qtpalette() function
in the new file qtpalette.c, which is shared by both matroskadec.c and
mov.c.

In matroskadec.c, I'm also putting the palette in 'extradata', like it's
done for V_MS/VFW/FOURCC; this is a requirement in order for MPlayer to
use the correct palette. And why is that, you may wonder. Well, it's
because for some mysterious reason, MPlayer adds *another* palette side
data packet *after* the one added in matroskadec.c. It uses whatever is
in extradata as the palette when adding this packet.

Video samples for testing are available at
https://drive.google.com/open?id=0B3_pEBoLs0faWElmM2FnLTZYNlk.

---
 libavformat/Makefile  |1 +
 libavformat/matroskadec.c |   32 +++-
 libavformat/mov.c |   78 -
 libavformat/qtpalette.c   |  121 +
 libavformat/qtpalette.h   |5 +-
 5 files changed, 166 insertions(+), 71 deletions(-)
 create mode 100644 libavformat/qtpalette.c

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 110e9e3..e03c73e 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -18,6 +18,7 @@ OBJS = allformats.o \
mux.o\
options.o\
os_support.o \
+   qtpalette.o  \
riff.o   \
sdp.o\
url.o\
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index c574749..9837a67 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -64,6 +64,8 @@
 #include 
 #endif
 
+#include "qtpalette.h"
+
 typedef enum {
 EBML_NONE,
 EBML_UINT,
@@ -312,6 +314,9 @@ typedef struct MatroskaDemuxContext {
 
 /* WebM DASH Manifest live flag/ */
 int is_live;
+
+uint32_t palette[AVPALETTE_COUNT];
+int has_palette;
 } MatroskaDemuxContext;
 
 typedef struct MatroskaBlock {
@@ -1856,7 +1861,7 @@ static int matroska_parse_tracks(AVFormatContext *s)
 fourcc   = st->codec->codec_tag;
 extradata_offset = FFMIN(track->codec_priv.size, 18);
 } else if (!strcmp(track->codec_id, "A_QUICKTIME")
-   && (track->codec_priv.size >= 86)
+   && (track->codec_priv.size >= 36)
&& (track->codec_priv.data)) {
 fourcc = AV_RL32(track->codec_priv.data + 4);
 codec_id = ff_codec_get_id(ff_codec_movaudio_tags, fourcc);
@@ -1881,6 +1886,22 @@ static int matroska_parse_tracks(AVFormatContext *s)
 av_log(matroska->ctx, AV_LOG_ERROR,
"mov FourCC not found %s.\n", buf);
 }
+if (track->codec_priv.size >= 86) {
+bit_depth = AV_RB16(track->codec_priv.data + 82);
+ffio_init_context(, track->codec_priv.data,
+  track->codec_priv.size,
+  0, 

Re: [FFmpeg-devel] [PATCH 11/15] lavc/on2avc: replace pow(10, x) by exp10(x)

[Ganesh Ajjanagadde]

On Sun, Dec 27, 2015 at 4:31 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Sat, Dec 26, 2015 at 8:23 PM, Ganesh Ajjanagadde 
> wrote:
>>
>> On Sat, Dec 26, 2015 at 4:20 PM, Ganesh Ajjanagadde 
>> wrote:
>> > On Sat, Dec 26, 2015 at 4:08 PM, James Almer  wrote:
>> >> On 12/23/2015 3:47 PM, Ganesh Ajjanagadde wrote:
>> >>> exp10, introduced recently, is superior for the purpose.
>> >>>
>> >>> Signed-off-by: Ganesh Ajjanagadde 
>> >>> ---
>> >>>  libavcodec/on2avc.c | 5 +++--
>> >>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
>> >>> index 04c8e41..0409b3e 100644
>> >>> --- a/libavcodec/on2avc.c
>> >>> +++ b/libavcodec/on2avc.c
>> >>> @@ -22,6 +22,7 @@
>> >>>
>> >>>  #include "libavutil/channel_layout.h"
>> >>>  #include "libavutil/float_dsp.h"
>> >>> +#include "libavutil/libm.h"
>> >>>  #include "avcodec.h"
>> >>>  #include "bytestream.h"
>> >>>  #include "fft.h"
>> >>> @@ -934,9 +935,9 @@ static av_cold int
>> >>> on2avc_decode_init(AVCodecContext *avctx)
>> >>> "Stereo mode support is not good, patch is
>> >>> welcome\n");
>> >>>
>> >>>  for (i = 0; i < 20; i++)
>> >>> -c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 16) / 32;
>> >>> +c->scale_tab[i] = ceil(exp10(i * 0.1) * 16) / 32;
>> >>>  for (; i < 128; i++)
>> >>> -c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 0.5);
>> >>> +c->scale_tab[i] = ceil(exp10(i * 0.1) * 0.5);
>> >>>
>> >>>  if (avctx->sample_rate < 32000 || avctx->channels == 1)
>> >>>  memcpy(c->long_win, ff_on2avc_window_long_24000,
>> >>
>> >> This apparently broke ICC
>> >>
>> >>
>> >> http://fate.ffmpeg.org/report.cgi?time=20151226215846=x86_64-linux-gnu-icc-2011.4.191
>> >>
>> >> http://fate.ffmpeg.org/report.cgi?time=20151226235348=x86_64-linux-gnu-icc-2011_sp1.13.367
>> >>
>> >> http://fate.ffmpeg.org/report.cgi?time=20151226203729=x86_64-archlinux-icc-2013
>> >
>> > Thanks for the report. A couple of questions:
>> > 1. Is there an easy way to acquire icc so that I can reproduce?
>> > GCC/Clang are perfectly fine with it.
>> > 2. Do you know what the ICC platforms use for exp2?
>> >
>> > In the absence of remarks and my own inability to fix this, I will
>> > revert tonight.
>> > BTW, please let me know the general policy for this kind of breakage:
>> > i.e, how quickly do such regressions need to be fixed.
>>
>> Different fix pushed that also speeds up things.
>
>
> You shouldn't push things without review...

I assumed this does not apply to things that were broken by commits
made by one self, and also assumed that regressions should be fixed
asap, proper fixes coming in later as Michael posted now.

Guess I was wrong here, noted this for the future. Thanks.

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

Re: [FFmpeg-devel] [PATCH 11/15] lavc/on2avc: replace pow(10, x) by exp10(x)

[Michael Niedermayer]

On Sun, Dec 27, 2015 at 08:00:52AM -0800, Ganesh Ajjanagadde wrote:
> On Sun, Dec 27, 2015 at 4:31 AM, Ronald S. Bultje  wrote:
> > Hi,
> >
> > On Sat, Dec 26, 2015 at 8:23 PM, Ganesh Ajjanagadde 
> > wrote:
> >>
> >> On Sat, Dec 26, 2015 at 4:20 PM, Ganesh Ajjanagadde 
> >> wrote:
> >> > On Sat, Dec 26, 2015 at 4:08 PM, James Almer  wrote:
> >> >> On 12/23/2015 3:47 PM, Ganesh Ajjanagadde wrote:
> >> >>> exp10, introduced recently, is superior for the purpose.
> >> >>>
> >> >>> Signed-off-by: Ganesh Ajjanagadde 
> >> >>> ---
> >> >>>  libavcodec/on2avc.c | 5 +++--
> >> >>>  1 file changed, 3 insertions(+), 2 deletions(-)
> >> >>>
> >> >>> diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
> >> >>> index 04c8e41..0409b3e 100644
> >> >>> --- a/libavcodec/on2avc.c
> >> >>> +++ b/libavcodec/on2avc.c
> >> >>> @@ -22,6 +22,7 @@
> >> >>>
> >> >>>  #include "libavutil/channel_layout.h"
> >> >>>  #include "libavutil/float_dsp.h"
> >> >>> +#include "libavutil/libm.h"
> >> >>>  #include "avcodec.h"
> >> >>>  #include "bytestream.h"
> >> >>>  #include "fft.h"
> >> >>> @@ -934,9 +935,9 @@ static av_cold int
> >> >>> on2avc_decode_init(AVCodecContext *avctx)
> >> >>> "Stereo mode support is not good, patch is
> >> >>> welcome\n");
> >> >>>
> >> >>>  for (i = 0; i < 20; i++)
> >> >>> -c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 16) / 32;
> >> >>> +c->scale_tab[i] = ceil(exp10(i * 0.1) * 16) / 32;
> >> >>>  for (; i < 128; i++)
> >> >>> -c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 0.5);
> >> >>> +c->scale_tab[i] = ceil(exp10(i * 0.1) * 0.5);
> >> >>>
> >> >>>  if (avctx->sample_rate < 32000 || avctx->channels == 1)
> >> >>>  memcpy(c->long_win, ff_on2avc_window_long_24000,
> >> >>
> >> >> This apparently broke ICC
> >> >>
> >> >>
> >> >> http://fate.ffmpeg.org/report.cgi?time=20151226215846=x86_64-linux-gnu-icc-2011.4.191
> >> >>
> >> >> http://fate.ffmpeg.org/report.cgi?time=20151226235348=x86_64-linux-gnu-icc-2011_sp1.13.367
> >> >>
> >> >> http://fate.ffmpeg.org/report.cgi?time=20151226203729=x86_64-archlinux-icc-2013
> >> >
> >> > Thanks for the report. A couple of questions:
> >> > 1. Is there an easy way to acquire icc so that I can reproduce?
> >> > GCC/Clang are perfectly fine with it.
> >> > 2. Do you know what the ICC platforms use for exp2?
> >> >
> >> > In the absence of remarks and my own inability to fix this, I will
> >> > revert tonight.
> >> > BTW, please let me know the general policy for this kind of breakage:
> >> > i.e, how quickly do such regressions need to be fixed.
> >>
> >> Different fix pushed that also speeds up things.
> >
> >
> > You shouldn't push things without review...
> 
> I assumed this does not apply to things that were broken by commits
> made by one self, and also assumed that regressions should be fixed
> asap, proper fixes coming in later as Michael posted now.
> 
> Guess I was wrong here, noted this for the future. Thanks.

IMO its ok to revert ones own commits if they break something

its also ok to commit things that everyone is happy with
like for example if you forget a ";" adding it is perfectly ok without
patch, noone will be unhappy about that.
the more complex a patch is and the more close it is to other peoples
authorship/maitaince the more likely someone else will be unhappy

sending patches vs. directly commiting so that work and latency is
minimized and everyone is happy is not always trivial

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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


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

Re: [FFmpeg-devel] [PATCH 11/15] lavc/on2avc: replace pow(10, x) by exp10(x)

[Ganesh Ajjanagadde]

On Sun, Dec 27, 2015 at 8:14 AM, Michael Niedermayer
 wrote:
> On Sun, Dec 27, 2015 at 08:00:52AM -0800, Ganesh Ajjanagadde wrote:
>> On Sun, Dec 27, 2015 at 4:31 AM, Ronald S. Bultje  wrote:
>> > Hi,
>> >
>> > On Sat, Dec 26, 2015 at 8:23 PM, Ganesh Ajjanagadde 
>> > wrote:
>> >>
>> >> On Sat, Dec 26, 2015 at 4:20 PM, Ganesh Ajjanagadde 
>> >> wrote:
>> >> > On Sat, Dec 26, 2015 at 4:08 PM, James Almer  wrote:
>> >> >> On 12/23/2015 3:47 PM, Ganesh Ajjanagadde wrote:
>> >> >>> exp10, introduced recently, is superior for the purpose.
>> >> >>>
>> >> >>> Signed-off-by: Ganesh Ajjanagadde 
>> >> >>> ---
>> >> >>>  libavcodec/on2avc.c | 5 +++--
>> >> >>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> >> >>>
>> >> >>> diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
>> >> >>> index 04c8e41..0409b3e 100644
>> >> >>> --- a/libavcodec/on2avc.c
>> >> >>> +++ b/libavcodec/on2avc.c
>> >> >>> @@ -22,6 +22,7 @@
>> >> >>>
>> >> >>>  #include "libavutil/channel_layout.h"
>> >> >>>  #include "libavutil/float_dsp.h"
>> >> >>> +#include "libavutil/libm.h"
>> >> >>>  #include "avcodec.h"
>> >> >>>  #include "bytestream.h"
>> >> >>>  #include "fft.h"
>> >> >>> @@ -934,9 +935,9 @@ static av_cold int
>> >> >>> on2avc_decode_init(AVCodecContext *avctx)
>> >> >>> "Stereo mode support is not good, patch is
>> >> >>> welcome\n");
>> >> >>>
>> >> >>>  for (i = 0; i < 20; i++)
>> >> >>> -c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 16) / 32;
>> >> >>> +c->scale_tab[i] = ceil(exp10(i * 0.1) * 16) / 32;
>> >> >>>  for (; i < 128; i++)
>> >> >>> -c->scale_tab[i] = ceil(pow(10.0, i * 0.1) * 0.5);
>> >> >>> +c->scale_tab[i] = ceil(exp10(i * 0.1) * 0.5);
>> >> >>>
>> >> >>>  if (avctx->sample_rate < 32000 || avctx->channels == 1)
>> >> >>>  memcpy(c->long_win, ff_on2avc_window_long_24000,
>> >> >>
>> >> >> This apparently broke ICC
>> >> >>
>> >> >>
>> >> >> http://fate.ffmpeg.org/report.cgi?time=20151226215846=x86_64-linux-gnu-icc-2011.4.191
>> >> >>
>> >> >> http://fate.ffmpeg.org/report.cgi?time=20151226235348=x86_64-linux-gnu-icc-2011_sp1.13.367
>> >> >>
>> >> >> http://fate.ffmpeg.org/report.cgi?time=20151226203729=x86_64-archlinux-icc-2013
>> >> >
>> >> > Thanks for the report. A couple of questions:
>> >> > 1. Is there an easy way to acquire icc so that I can reproduce?
>> >> > GCC/Clang are perfectly fine with it.
>> >> > 2. Do you know what the ICC platforms use for exp2?
>> >> >
>> >> > In the absence of remarks and my own inability to fix this, I will
>> >> > revert tonight.
>> >> > BTW, please let me know the general policy for this kind of breakage:
>> >> > i.e, how quickly do such regressions need to be fixed.
>> >>
>> >> Different fix pushed that also speeds up things.
>> >
>> >
>> > You shouldn't push things without review...
>>
>> I assumed this does not apply to things that were broken by commits
>> made by one self, and also assumed that regressions should be fixed
>> asap, proper fixes coming in later as Michael posted now.
>>
>> Guess I was wrong here, noted this for the future. Thanks.
>
> IMO its ok to revert ones own commits if they break something

thanks for this statement: what I did was not a simple revert, so I
guess I should have asked.

>
> its also ok to commit things that everyone is happy with
> like for example if you forget a ";" adding it is perfectly ok without
> patch, noone will be unhappy about that.
> the more complex a patch is and the more close it is to other peoples
> authorship/maitaince the more likely someone else will be unhappy
>
> sending patches vs. directly commiting so that work and latency is
> minimized and everyone is happy is not always trivial

There is only one thing I am a bit unhappy about here: the patch you
pushed: since it was related to code I write, and I am active on the
ML here, I would have appreciated at least obtaining my opinion on the
patch before pushing.

It would have still been a nack, but as it is not technically wrong, I
would not have opposed it if someone else acks it, like Ronald in this
case.

I guess you and Ronald are unhappy with what I did to fix the broken
change. Overall, seems like simple miscommunication, and everything is
back to normal here.

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I have never wished to cater to the crowd; for what I know they do not
> approve, and what they approve I do not know. -- Epicurus
>
> ___
> 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 2/3] avformat/hls: Remember to free HLSContext::headers

[Michael Niedermayer]

On Sun, Dec 27, 2015 at 12:21:44PM +, Joel Holdsworth wrote:
> ---
>  libavformat/hls.c | 1 +
>  1 file changed, 1 insertion(+)

applied

thanks

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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras


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

Re: [FFmpeg-devel] [PATCH 11/15] lavc/on2avc: replace pow(10, x) by exp10(x)

[Ronald S. Bultje]

Hi,

On Sun, Dec 27, 2015 at 11:32 AM, Ganesh Ajjanagadde 
wrote:

> I guess you and Ronald are unhappy with what I did to fix the broken
> change. Overall, seems like simple miscommunication, and everything is
> back to normal here.


Back-to-normal sounds about right :)

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

Re: [FFmpeg-devel] [PATCH 1/3] avformat/http: Added http_proxy option

[Michael Niedermayer]

On Sun, Dec 27, 2015 at 12:21:43PM +, Joel Holdsworth wrote:
> ---
>  libavformat/http.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

applied

thanks

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

There will always be a question for which you do not know the correct answer.


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

Re: [FFmpeg-devel] [PATCH] avcodec/on2avc: Fix stability issues with scale_tab generation

[Ganesh Ajjanagadde]

On Sun, Dec 27, 2015 at 7:44 AM, Michael Niedermayer
 wrote:
> On Sun, Dec 27, 2015 at 07:40:53AM -0500, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Sun, Dec 27, 2015 at 6:02 AM, Michael Niedermayer 
>> wrote:
>>
>> > +for (i = 0; i < 20; i++)
>> > +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 16 - 0.01) / 32;
>> >  for (; i < 128; i++)
>> > +c->scale_tab[i] = ceil(ff_exp10(i * 0.1) * 0.5 - 0.01);
>> >
>>
>> To innocent bystanders, this is hard to understand. Let's keep it a habit
>> to document things (what and why), where the "what" portion is probably
>> "this code emulates pow(10, x) with ff_pow10(x - 0.01)". As for why (most
>> specifically, why the 0.01?), I'm going to assume that here, you're trying
>> to get unit integers to not go to unit.000[..]001, so you subtract 0.01
>> before the ceil so it works fine again to get the exact unit integer output
>> number. If that's correct, please add a comment saying that, and then lgtm.

In my view this change is not good for the exact reasons above. It is
a "magic constant" that is a pure hack. The commit I made made
accuracy more consistent across platforms (instead of relying on the
whims of libm's pow/exp2), and was commented so that intent was clear.
Nack from me.

>
> yes
> changed
> applied
> thx
>
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Concerning the gods, I have no means of knowing whether they exist or not
> or of what sort they may be, because of the obscurity of the subject, and
> the brevity of human life -- Protagoras
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec/utils: fix check for invalid timebase when decoding subtitles

[Hendrik Leppkes]

Invalid timebases have a zero numerator, not denominator.
Fixes a integer divison by zero.
---
 libavcodec/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 19f3f0a..33295ed 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2435,7 +2435,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
AVSubtitle *sub,
 } else {
 avctx->internal->pkt = _recoded;
 
-if (avctx->pkt_timebase.den && avpkt->pts != AV_NOPTS_VALUE)
+if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE)
 sub->pts = av_rescale_q(avpkt->pts,
 avctx->pkt_timebase, AV_TIME_BASE_Q);
 ret = avctx->codec->decode(avctx, sub, got_sub_ptr, _recoded);
-- 
2.6.2.windows.1

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

Re: [FFmpeg-devel] [PATCH 3/3] avformat/hls: Added http_proxy support

[Michael Niedermayer]

On Sun, Dec 27, 2015 at 12:21:45PM +, Joel Holdsworth wrote:
> ---
>  libavformat/hls.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

applied

thanks

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

You can kill me, but you cannot change the truth.


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

Re: [FFmpeg-devel] [PATCH] avcodec/utils: fix check for invalid timebase when decoding subtitles

[Hendrik Leppkes]

On Sun, Dec 27, 2015 at 9:03 PM, Andreas Cadhalpun
 wrote:
> On 27.12.2015 20:43, Hendrik Leppkes wrote:
>> On Sun, Dec 27, 2015 at 8:29 PM, Andreas Cadhalpun
>>  wrote:
>>> On 27.12.2015 20:10, Hendrik Leppkes wrote:
 Invalid timebases have a zero numerator, not denominator.
>>>
>>> A timebase with zero numerator is probably invalid, but a timebase
>>> with zero denominator is not even well defined.
>>> So this comment doesn't seem quite right.
>>>
 Fixes a integer divison by zero.
 ---
  libavcodec/utils.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/libavcodec/utils.c b/libavcodec/utils.c
 index 19f3f0a..33295ed 100644
 --- a/libavcodec/utils.c
 +++ b/libavcodec/utils.c
 @@ -2435,7 +2435,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
 AVSubtitle *sub,
  } else {
  avctx->internal->pkt = _recoded;

 -if (avctx->pkt_timebase.den && avpkt->pts != AV_NOPTS_VALUE)
 +if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE)
  sub->pts = av_rescale_q(avpkt->pts,
  avctx->pkt_timebase, 
 AV_TIME_BASE_Q);
  ret = avctx->codec->decode(avctx, sub, got_sub_ptr, 
 _recoded);

>>>
>>> If avctx->pkt_timebase.den is 0, calling av_rescale_q here will trigger the
>>> av_assert2(c > 0) in av_rescale_rnd, so removing this check isn't good.
>>>
>>> Why not check for both num and den?
>>>
>>
>> We never check both, invalid timebase is {0, 1}, anything else needs
>> to have values in both.
>
> I'd call this unknown timebase, as {0, 1} is the initialization value.
> A {1, 0} timebase is certainly not valid.
>
>> All timebase checks are for num only.
>
> The check here was for den and there is a similar check in 
> teletext_decode_frame.

And thats a bug since that can actually lead to div by 0 and crash.
The same timebase is checked 5 lines down for num already.

>
>> Its an assert2 for a reason (ie. a developer tool), its checked in an
>> error condition right after and returns INT64_MIN.
>
> It's an assert, because it should never happen.
>
> If the check for den here was intended to prevent triggering such an assert,
> then it would still be needed.
> If on the other hand this check was meant to check for an uninitialized
> pkt_timebase, then removing it would be fine.
>
> So you can remove this check, if you're sure it's the second case.
> But please clarify this in the commit message.
>
> Best regards,
> Andreas
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat: stop exporting ffurl_read_complete, ffurl_seek and ffurl_size

[Andreas Cadhalpun]

On 27.12.2015 19:32, Jan Ehrhardt wrote:
> Andreas Cadhalpun in gmane.comp.video.ffmpeg.devel (Tue, 27 Oct 2015
> 22:52:27 +0100):
>>> On Tue, Oct 27, 2015 at 9:58 PM, Andreas Cadhalpun
 diff --git a/libavformat/libavformat.v b/libavformat/libavformat.v
 index e90aef7..a00a309 100644
 --- a/libavformat/libavformat.v
 +++ b/libavformat/libavformat.v
 @@ -10,9 +10,6 @@ LIBAVFORMAT_$MAJOR {
 -ffurl_read_complete;
 -ffurl_seek;
 -ffurl_size;
>>
>> OK. I'll wait a bit to see if someone else wants to comment on this patch.
> 
> There is a PHP extension that uses ffurl_seek and ffurl_read_complete:
> https://github.com/chung-leong/av/blob/master/faststart.c#L76

This extension is apparently not developed anymore: the last commit was about
two years ago.

> These functions are also documented at
> http://www.ffmpeg.org/doxygen/trunk/avio_8c.html

This documentation contains every (public or private) function of ffmpeg.

> How should an external developer know what functions are publically
> available?

Publicly available functions are declared in installed headers, which
ffurl_seek and ffurl_read have never been.
Whoever wrote this extension must have been aware of the fact that these
are private functions as they are explicitly declared in the code [1]:
// from libavformat/url.h

typedef struct URLContext URLContext;
int ffurl_read_complete(URLContext *h, unsigned char *buf, int size);
int ffurl_write(URLContext *h, const unsigned char *buf, int size);
int64_t ffurl_seek(URLContext *h, int64_t pos, int whence);

This shouldn't have been done in the first place...

Best regards,
Andreas


1: https://github.com/chung-leong/av/blob/master/faststart.c#L51
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/utils: fix check for invalid timebase when decoding subtitles

[Andreas Cadhalpun]

On 27.12.2015 21:13, Hendrik Leppkes wrote:
> On Sun, Dec 27, 2015 at 9:03 PM, Andreas Cadhalpun
>  wrote:
>> On 27.12.2015 20:43, Hendrik Leppkes wrote:
>>> On Sun, Dec 27, 2015 at 8:29 PM, Andreas Cadhalpun
>>>  wrote:
 On 27.12.2015 20:10, Hendrik Leppkes wrote:
> Invalid timebases have a zero numerator, not denominator.

 A timebase with zero numerator is probably invalid, but a timebase
 with zero denominator is not even well defined.
 So this comment doesn't seem quite right.

> Fixes a integer divison by zero.
> ---
>  libavcodec/utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 19f3f0a..33295ed 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2435,7 +2435,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, 
> AVSubtitle *sub,
>  } else {
>  avctx->internal->pkt = _recoded;
>
> -if (avctx->pkt_timebase.den && avpkt->pts != AV_NOPTS_VALUE)
> +if (avctx->pkt_timebase.num && avpkt->pts != AV_NOPTS_VALUE)
>  sub->pts = av_rescale_q(avpkt->pts,
>  avctx->pkt_timebase, 
> AV_TIME_BASE_Q);
>  ret = avctx->codec->decode(avctx, sub, got_sub_ptr, 
> _recoded);
>

 If avctx->pkt_timebase.den is 0, calling av_rescale_q here will trigger the
 av_assert2(c > 0) in av_rescale_rnd, so removing this check isn't good.

 Why not check for both num and den?

>>>
>>> We never check both, invalid timebase is {0, 1}, anything else needs
>>> to have values in both.
>>
>> I'd call this unknown timebase, as {0, 1} is the initialization value.
>> A {1, 0} timebase is certainly not valid.
>>
>>> All timebase checks are for num only.
>>
>> The check here was for den and there is a similar check in 
>> teletext_decode_frame.
> 
> And thats a bug since that can actually lead to div by 0 and crash.

Then please fix this bug also in teletext_decode_frame.

> The same timebase is checked 5 lines down for num already.

Indeed.

Best regards,
Andreas

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

[FFmpeg-devel] ffplay: insertion point of the auto rotation filter - Github ticket #141

[Balint Marton]

Hi Michael,

The patch you committed seems to break the cropping to even width / height 
as required by SDL overlay code.


Also there can be a use case for inserting the auto rotation filter after 
the user provided filter chain as well. (e,g, deinterlacing).


I don't think we can make everyone happy, so unless you have a better 
idea, I think it would be best if you could just revert the patch.


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

[FFmpeg-devel] [PATCH 1/2] lavfi/af_anequalizer: replace pow(10, x) by ff_exp10(x)

[Ganesh Ajjanagadde]

Signed-off-by: Ganesh Ajjanagadde 
---
 libavfilter/af_anequalizer.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/libavfilter/af_anequalizer.c b/libavfilter/af_anequalizer.c
index e45c108..d7b5b6c 100644
--- a/libavfilter/af_anequalizer.c
+++ b/libavfilter/af_anequalizer.c
@@ -316,9 +316,9 @@ static void butterworth_bp_filter(EqualizatorFilter *f,
 return;
 }
 
-G  = pow(10, G/20);
-Gb = pow(10, Gb/20);
-G0 = pow(10, G0/20);
+G  = ff_exp10(G/20);
+Gb = ff_exp10(Gb/20);
+G0 = ff_exp10(G0/20);
 
 epsilon = sqrt((G * G - Gb * Gb) / (Gb * Gb - G0 * G0));
 g  = pow(G,  1.0 / N);
@@ -385,9 +385,9 @@ static void chebyshev1_bp_filter(EqualizatorFilter *f,
 return;
 }
 
-G  = pow(10, G/20);
-Gb = pow(10, Gb/20);
-G0 = pow(10, G0/20);
+G  = ff_exp10(G/20);
+Gb = ff_exp10(Gb/20);
+G0 = ff_exp10(G0/20);
 
 epsilon = sqrt((G*G - Gb*Gb) / (Gb*Gb - G0*G0));
 g0 = pow(G0,1.0/N);
@@ -458,9 +458,9 @@ static void chebyshev2_bp_filter(EqualizatorFilter *f,
 return;
 }
 
-G  = pow(10, G/20);
-Gb = pow(10, Gb/20);
-G0 = pow(10, G0/20);
+G  = ff_exp10(G/20);
+Gb = ff_exp10(Gb/20);
+G0 = ff_exp10(G0/20);
 
 epsilon = sqrt((G*G - Gb*Gb) / (Gb*Gb - G0*G0));
 g  = pow(G, 1.0 / N);
-- 
2.6.4

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

[FFmpeg-devel] [PATCH 2/2] lavfi/af_anequalizer: replace pow(x, -2) by 1/(x*x)

[Ganesh Ajjanagadde]

Signed-off-by: Ganesh Ajjanagadde 
---
 libavfilter/af_anequalizer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavfilter/af_anequalizer.c b/libavfilter/af_anequalizer.c
index d7b5b6c..649c0b9 100644
--- a/libavfilter/af_anequalizer.c
+++ b/libavfilter/af_anequalizer.c
@@ -391,8 +391,8 @@ static void chebyshev1_bp_filter(EqualizatorFilter *f,
 
 epsilon = sqrt((G*G - Gb*Gb) / (Gb*Gb - G0*G0));
 g0 = pow(G0,1.0/N);
-alfa = pow(1.0/epsilon+ sqrt(1 + pow(epsilon,-2.0)), 1.0/N);
-beta = pow(G/epsilon + Gb * sqrt(1 + pow(epsilon,-2.0)), 1.0/N);
+alfa = pow(1.0/epsilon+ sqrt(1 + 1/(epsilon*epsilon)), 1.0/N);
+beta = pow(G/epsilon + Gb * sqrt(1 + 1/(epsilon*epsilon)), 1.0/N);
 a = 0.5 * (alfa - 1.0/alfa);
 b = 0.5 * (beta - g0*g0*(1/beta));
 tetta_b = tan(wb/2);
-- 
2.6.4

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

Re: [FFmpeg-devel] [PATCH]ffserver: Cast time_t value when using it in a format string.

[Reynaldo H. Verdejo Pinochet]

Looks good.

On 12/22/2015 12:40 AM, Carl Eugen Hoyos wrote:
> Hi!
> 
> Attached patch should fix ticket #5103.
> 
> Please comment, Carl Eugen
> 
> 
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

-- 
Reynaldo H. Verdejo Pinochet
Open Source Group
Samsung Research America / Silicon Valley
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 02:56 AM, Mats Peterson wrote:

On 12/28/2015 01:24 AM, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:


+memcpy(st->codec->extradata, matroska->palette,
+AVPALETTE_SIZE);


As said, please remove this, you must not fix MPlayer
issues in FFmpeg.
(The issue in MPlayer does not exist but that doesn't
matter on this mailing list.)

Carl Eugen

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



It's not you who decides this, but Michael Niedermayer. And there is
nothing wrong about putting the palette in extradata, at that. I won't
change it unless Michael tells me to. Aren't you the bug tracker
maintainer, by the way?

Mats



"The issue doesn't exist". You obviously haven't tried it. Now please 
stop harassing me, and leave this to Michael.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files

[Eran Kornblau]

Bumping up this thread (latest patch attached)

Happy holidays !

Eran



0001-mov-support-cenc-common-encryption.patch
Description: 0001-mov-support-cenc-common-encryption.patch
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] Write correct creation time of new ASF

[Vadim Belov]

---
 libavformat/asfenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/asfenc.c b/libavformat/asfenc.c
index 32b726b..9bb439a 100644
--- a/libavformat/asfenc.c
+++ b/libavformat/asfenc.c
@@ -428,7 +428,7 @@ static int asf_write_header1(AVFormatContext *s, int64_t 
file_size,
 hpos  = put_header(pb, _asf_file_header);
 ff_put_guid(pb, _asf_my_guid);
 avio_wl64(pb, file_size);
-file_time = 0;
+file_time = time(NULL);
 avio_wl64(pb, unix_to_file_time(file_time));
 avio_wl64(pb, asf->nb_packets); /* number of packets */
 avio_wl64(pb, duration); /* end time stamp (in 100ns units) */
-- 
2.6.4.windows.1

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

[FFmpeg-devel] [PATCH 2/2] x86/vf_stereo3d: make ff_anaglyph_sse4 work on x86_32

[James Almer]

Signed-off-by: James Almer 
---
 libavfilter/x86/vf_stereo3d.asm| 47 +++---
 libavfilter/x86/vf_stereo3d_init.c |  2 +-
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/libavfilter/x86/vf_stereo3d.asm b/libavfilter/x86/vf_stereo3d.asm
index 29a8c56..491579f 100644
--- a/libavfilter/x86/vf_stereo3d.asm
+++ b/libavfilter/x86/vf_stereo3d.asm
@@ -22,8 +22,6 @@
 
 %include "libavutil/x86/x86util.asm"
 
-%if ARCH_X86_64
-
 SECTION_RODATA
 
 ; rgbrgbrgbrgb
@@ -37,10 +35,33 @@ ex_b: db 2,-1,-1,-1,5,-1,-1,-1,8,-1,-1,-1,11,-1,-1,-1
 SECTION .text
 
 INIT_XMM sse4
+%if ARCH_X86_64
 cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, rsrc, dst_linesize, 
l_linesize, r_linesize, width, height, o, cnt
 %define ana_matrix_rq r6q
 %define ana_matrix_gq r7q
 %define ana_matrix_bq r8q
+
+%else ; ARCH_X86_32
+%if HAVE_ALIGNED_STACK
+cglobal anaglyph, 3, 7, 8, 2*9*mmsize, dst, lsrc, rsrc, dst_linesize, 
l_linesize, o, cnt
+%else
+cglobal anaglyph, 3, 6, 8, 2*9*mmsize, dst, lsrc, rsrc, dst_linesize, o, cnt
+%define l_linesizeq r4mp
+%endif ; HAVE_ALIGNED_STACK
+%define ana_matrix_rq r3q
+%define ana_matrix_gq r4q
+%define ana_matrix_bq r5q
+%define r_linesizeq r5mp
+%define widthd  r6mp
+%define heightd r7mp
+%define  m8 [rsp+mmsize*12]
+%define  m9 [rsp+mmsize*13]
+%define m10 [rsp+mmsize*14]
+%define m11 [rsp+mmsize*15]
+%define m12 [rsp+mmsize*16]
+%define m13 [rsp+mmsize*17]
+%endif ; ARCH
+
 movana_matrix_rq, r8m
 movana_matrix_gq, r9m
 movana_matrix_bq, r10m
@@ -74,6 +95,7 @@ cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, rsrc, 
dst_linesize, l_linesi
 mova [rsp+mmsize*10], m4
 mova [rsp+mmsize*11], m5
 
+%if ARCH_X86_64
 movu m11, [ana_matrix_bq+ 0]
 movq m13, [ana_matrix_bq+16]
 pshufdm8, m11, q
@@ -84,6 +106,26 @@ cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, rsrc, 
dst_linesize, l_linesi
 pshufd   m13, m13, q
 mov   widthd, dword widthm
 mov  heightd, dword heightm
+%else
+movu  m3, [ana_matrix_bq+ 0]
+movq  m5, [ana_matrix_bq+16]
+pshufdm0, m3, q
+pshufdm1, m3, q
+pshufdm2, m3, q
+pshufdm3, m3, q
+pshufdm4, m5, q
+pshufdm5, m5, q
+mova [rsp+mmsize*12], m0
+mova [rsp+mmsize*13], m1
+mova [rsp+mmsize*14], m2
+mova [rsp+mmsize*15], m3
+mova [rsp+mmsize*16], m4
+mova [rsp+mmsize*17], m5
+movdst_linesizeq, r3m
+%if HAVE_ALIGNED_STACK
+mov  l_linesizeq, r4m
+%endif
+%endif ; ARCH
 
 .nextrow:
 mov   od, widthd
@@ -172,4 +214,3 @@ cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, rsrc, 
dst_linesize, l_linesi
 sub   heightd, 1
 jg .nextrow
 REP_RET
-%endif
diff --git a/libavfilter/x86/vf_stereo3d_init.c 
b/libavfilter/x86/vf_stereo3d_init.c
index 77d4f7b..da160a8 100644
--- a/libavfilter/x86/vf_stereo3d_init.c
+++ b/libavfilter/x86/vf_stereo3d_init.c
@@ -31,7 +31,7 @@ void ff_stereo3d_init_x86(Stereo3DDSPContext *dsp)
 {
 int cpu_flags = av_get_cpu_flags();
 
-if (ARCH_X86_64 && EXTERNAL_SSE4(cpu_flags)) {
+if (EXTERNAL_SSE4(cpu_flags)) {
 dsp->anaglyph = ff_anaglyph_sse4;
 }
 }
-- 
2.6.3

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

[FFmpeg-devel] [PATCH 1/2] x86/vf_stereo3d: optimize register usage

[James Almer]

Signed-off-by: James Almer 
---
 libavfilter/x86/vf_stereo3d.asm | 164 +---
 1 file changed, 86 insertions(+), 78 deletions(-)

diff --git a/libavfilter/x86/vf_stereo3d.asm b/libavfilter/x86/vf_stereo3d.asm
index 94a0473..29a8c56 100644
--- a/libavfilter/x86/vf_stereo3d.asm
+++ b/libavfilter/x86/vf_stereo3d.asm
@@ -37,125 +37,133 @@ ex_b: db 2,-1,-1,-1,5,-1,-1,-1,8,-1,-1,-1,11,-1,-1,-1
 SECTION .text
 
 INIT_XMM sse4
-cglobal anaglyph, 11, 13, 16, 2*6*mmsize, dst, lsrc, rsrc, dst_linesize, 
l_linesize, r_linesize, width, height, ana_matrix_r, ana_matrix_g, ana_matrix_b
-movu m13, [ana_matrix_rq+ 0]
-movq m15, [ana_matrix_rq+16]
-pshufd   m10, m13, q
-pshufd   m11, m13, q
-pshufd   m12, m13, q
-pshufd   m13, m13, q
-pshufd   m14, m15, q
-pshufd   m15, m15, q
-mova  [rsp+mmsize*0], m10
-mova  [rsp+mmsize*1], m11
-mova  [rsp+mmsize*2], m12
-mova  [rsp+mmsize*3], m13
-mova  [rsp+mmsize*4], m14
-mova  [rsp+mmsize*5], m15
-
-movu m13, [ana_matrix_gq+ 0]
-movq m15, [ana_matrix_gq+16]
-pshufd   m10, m13, q
-pshufd   m11, m13, q
-pshufd   m12, m13, q
-pshufd   m13, m13, q
-pshufd   m14, m15, q
-pshufd   m15, m15, q
-mova [rsp+mmsize*6 ], m10
-mova [rsp+mmsize*7 ], m11
-mova [rsp+mmsize*8 ], m12
-mova [rsp+mmsize*9 ], m13
-mova [rsp+mmsize*10], m14
-mova [rsp+mmsize*11], m15
-
-movu m13, [ana_matrix_bq+ 0]
-movq m15, [ana_matrix_bq+16]
-pshufd   m10, m13, q
-pshufd   m11, m13, q
-pshufd   m12, m13, q
-pshufd   m13, m13, q
-pshufd   m14, m15, q
-pshufd   m15, m15, q
+cglobal anaglyph, 6, 10, 14, 2*6*mmsize, dst, lsrc, rsrc, dst_linesize, 
l_linesize, r_linesize, width, height, o, cnt
+%define ana_matrix_rq r6q
+%define ana_matrix_gq r7q
+%define ana_matrix_bq r8q
+movana_matrix_rq, r8m
+movana_matrix_gq, r9m
+movana_matrix_bq, r10m
+movu  m3, [ana_matrix_rq+ 0]
+movq  m5, [ana_matrix_rq+16]
+pshufdm0, m3, q
+pshufdm1, m3, q
+pshufdm2, m3, q
+pshufdm3, m3, q
+pshufdm4, m5, q
+pshufdm5, m5, q
+mova  [rsp+mmsize*0], m0
+mova  [rsp+mmsize*1], m1
+mova  [rsp+mmsize*2], m2
+mova  [rsp+mmsize*3], m3
+mova  [rsp+mmsize*4], m4
+mova  [rsp+mmsize*5], m5
+
+movu  m3, [ana_matrix_gq+ 0]
+movq  m5, [ana_matrix_gq+16]
+pshufdm0, m3, q
+pshufdm1, m3, q
+pshufdm2, m3, q
+pshufdm3, m3, q
+pshufdm4, m5, q
+pshufdm5, m5, q
+mova [rsp+mmsize*6 ], m0
+mova [rsp+mmsize*7 ], m1
+mova [rsp+mmsize*8 ], m2
+mova [rsp+mmsize*9 ], m3
+mova [rsp+mmsize*10], m4
+mova [rsp+mmsize*11], m5
+
+movu m11, [ana_matrix_bq+ 0]
+movq m13, [ana_matrix_bq+16]
+pshufdm8, m11, q
+pshufdm9, m11, q
+pshufd   m10, m11, q
+pshufd   m11, m11, q
+pshufd   m12, m13, q
+pshufd   m13, m13, q
+mov   widthd, dword widthm
+mov  heightd, dword heightm
+
 .nextrow:
-mov   r11q, widthq
-mov   r12q, 0
-%define  o  r12q
+mov   od, widthd
+xor cntd, cntd
 
 .loop:
-movu m0, [lsrcq+o+0]
+movu m0, [lsrcq+cntq]
 pshufb   m1, m0, [ex_r]
 pshufb   m2, m0, [ex_g]
 pshufb   m3, m0, [ex_b]
-movu m0, [rsrcq+o+0]
+movu m0, [rsrcq+cntq]
 pshufb   m4, m0, [ex_r]
 pshufb   m5, m0, [ex_g]
-pshufb   m6, m0, [ex_b]
+pshufb   m0, [ex_b]
 pmulld   m1, [rsp+mmsize*0]
 pmulld   m2, [rsp+mmsize*1]
 pmulld   m3, [rsp+mmsize*2]
 pmulld   m4, [rsp+mmsize*3]
 pmulld   m5, [rsp+mmsize*4]
-pmulld   m6, [rsp+mmsize*5]
+pmulld   m0, [rsp+mmsize*5]
 paddd

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Carl Eugen Hoyos]

Michael Niedermayer  niedermayer.cc> writes:

> Patch splited in move and matroska part
> i removed this memcpy() for now from what i 
> commited as there is clearly no consenus on it

I would really have appreciated a real review:
Apart from the unrelated audio fix a significant 
part of the patch was written by me.

And I suspect remuxing does not work yet, but 
I can only test next week.

Carl Eugen

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

[FFmpeg-devel] [PATCH] lavf/matroskadec.c: Copy QuickTime palette to extradata

[Mats Peterson]

I still insist on copying the QuickTime palette to extradata in
matroskadec.c, since it's currently needed for MPlayer to use the
correct palette. As I have explained so many times before, MPlayer, for
some inexplicable reason, currently adds *another* palette side data
packet *after* the one added in matroskadec.c, using whatever is in
extradata as the palette.

Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
>From e6202f0f7caa9c82915b2fa3276b263d16d1 Mon Sep 17 00:00:00 2001
From: Mats Peterson 
Date: Mon, 28 Dec 2015 05:12:49 +0100
Subject: [PATCH] lavf/matroskadec.c: Copy QuickTime palette to extradata

I still insist on copying the QuickTime palette to extradata in
matroskadec.c, since it's currently needed for MPlayer to use the
correct palette. As I have explained so many times before, MPlayer, for
some inexpblicable reason, currently adds *another* palette side data
packet *after* the one added in matroskadec.c, using whatever is in
extradata as the palette.

Mats

---
 libavformat/matroskadec.c |9 +
 1 file changed, 9 insertions(+)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 9de7cfb..d788dce 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1893,6 +1893,15 @@ static int matroska_parse_tracks(AVFormatContext *s)
   0, NULL, NULL, NULL, NULL);
 if (ff_get_qtpalette(codec_id, , matroska->palette)) {
 bit_depth &= 0x1F;
+/* Copy the palette to extradata. This is needed
+ * because MPlayer currently adds *another* palette
+ * side data packet *after* the one added in
+ * matroska_deliver_packet(), using whatever is in
+ * extradata as the palette. */
+if (ff_alloc_extradata(st->codec, AVPALETTE_SIZE))
+return AVERROR(ENOMEM);
+memcpy(st->codec->extradata, matroska->palette,
+AVPALETTE_SIZE);
 matroska->has_palette = 1;
 }
 }
-- 
1.7.10.4

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

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 04:12 AM, Mats Peterson wrote:

On 12/28/2015 04:10 AM, Carl Eugen Hoyos wrote:

Michael Niedermayer  niedermayer.cc> writes:


Patch splited in move and matroska part
i removed this memcpy() for now from what i
commited as there is clearly no consenus on it


I would really have appreciated a real review:
Apart from the unrelated audio fix a significant
part of the patch was written by me.

And I suspect remuxing does not work yet, but
I can only test next week.

Carl Eugen

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



Which part was written by you? I wonder because I have changed most of
what was the old patch.

Mats



Remuxing works fine, for the record.

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 04:10 AM, Carl Eugen Hoyos wrote:

Michael Niedermayer  niedermayer.cc> writes:


Patch splited in move and matroska part
i removed this memcpy() for now from what i
commited as there is clearly no consenus on it


I would really have appreciated a real review:
Apart from the unrelated audio fix a significant
part of the patch was written by me.

And I suspect remuxing does not work yet, but
I can only test next week.

Carl Eugen

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



Which part was written by you? I wonder because I have changed most of 
what was the old patch.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 02:58 AM, Mats Peterson wrote:

On 12/28/2015 02:56 AM, Mats Peterson wrote:

On 12/28/2015 01:24 AM, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:


+memcpy(st->codec->extradata, matroska->palette,
+AVPALETTE_SIZE);


As said, please remove this, you must not fix MPlayer
issues in FFmpeg.
(The issue in MPlayer does not exist but that doesn't
matter on this mailing list.)

Carl Eugen

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



It's not you who decides this, but Michael Niedermayer. And there is
nothing wrong about putting the palette in extradata, at that. I won't
change it unless Michael tells me to. Aren't you the bug tracker
maintainer, by the way?

Mats



"The issue doesn't exist". You obviously haven't tried it. Now please
stop harassing me, and leave this to Michael.

Mats



You're just making a fool out of yourself by acting like this, Eugen.

Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 03:21 AM, Mats Peterson wrote:

On 12/28/2015 03:16 AM, Michael Niedermayer wrote:

On Mon, Dec 28, 2015 at 12:24:10AM +, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:


+memcpy(st->codec->extradata, matroska->palette,
+AVPALETTE_SIZE);





As said, please remove this,



you must not fix MPlayer
issues in FFmpeg.


this is true but iam not sure if this is really "just" a mplayer
issue.
If there is a global palette, then it is not unreasonable that it
is placed in a stream global place. This is especially true if not
all packets are copied and the palette from the first packet is
lost as a result.
either way, the commit message for this part just points at mplayer,
which isnt really good for a description of a "bug fix" in libavformat

Patch splited in move and matroska part
i removed this memcpy() for now from what i commited as there is
clearly no consenus on it.

Michael, the concensus is that MPlayer won't work without having the
palette in extradata, since it uses the extradata as the palette when it
adds ANOTHER palette side data packet AFTER the one added in
matroskadec.c. I'm not making this up. Carl is of course right in his
statement that we shouldn't fix MPlayer issues in FFmpeg, but perhaps it
could be a "quick fix" until the MPlayer developers wake up?

Mats

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


Especially since MPlayer is such an evident use case of FFmpeg. It even 
includes it inside its own source code.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 01:24 AM, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:


+memcpy(st->codec->extradata, matroska->palette,
+AVPALETTE_SIZE);


As said, please remove this, you must not fix MPlayer
issues in FFmpeg.
(The issue in MPlayer does not exist but that doesn't
matter on this mailing list.)

Carl Eugen

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



It's not you who decides this, but Michael Niedermayer. And there is 
nothing wrong about putting the palette in extradata, at that. I won't 
change it unless Michael tells me to. Aren't you the bug tracker 
maintainer, by the way?


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Michael Niedermayer]

On Mon, Dec 28, 2015 at 12:24:10AM +, Carl Eugen Hoyos wrote:
> Mats Peterson  ffmpeg.org> writes:
> 
> > +memcpy(st->codec->extradata, matroska->palette,
> > +AVPALETTE_SIZE);
> 

> As said, please remove this,

> you must not fix MPlayer
> issues in FFmpeg.

this is true but iam not sure if this is really "just" a mplayer
issue.
If there is a global palette, then it is not unreasonable that it
is placed in a stream global place. This is especially true if not
all packets are copied and the palette from the first packet is
lost as a result.
either way, the commit message for this part just points at mplayer,
which isnt really good for a description of a "bug fix" in libavformat

Patch splited in move and matroska part
i removed this memcpy() for now from what i commited as there is
clearly no consenus on it


> (The issue in MPlayer does not exist but that doesn't 
> matter on this mailing list.)
> 
> Carl Eugen
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


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

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 03:27 AM, Mats Peterson wrote:

On 12/28/2015 03:21 AM, Mats Peterson wrote:

On 12/28/2015 03:16 AM, Michael Niedermayer wrote:

On Mon, Dec 28, 2015 at 12:24:10AM +, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:


+memcpy(st->codec->extradata, matroska->palette,
+AVPALETTE_SIZE);





As said, please remove this,



you must not fix MPlayer
issues in FFmpeg.


this is true but iam not sure if this is really "just" a mplayer
issue.
If there is a global palette, then it is not unreasonable that it
is placed in a stream global place. This is especially true if not
all packets are copied and the palette from the first packet is
lost as a result.
either way, the commit message for this part just points at mplayer,
which isnt really good for a description of a "bug fix" in libavformat

Patch splited in move and matroska part
i removed this memcpy() for now from what i commited as there is
clearly no consenus on it.

Michael, the concensus is that MPlayer won't work without having the
palette in extradata, since it uses the extradata as the palette when it
adds ANOTHER palette side data packet AFTER the one added in
matroskadec.c. I'm not making this up. Carl is of course right in his
statement that we shouldn't fix MPlayer issues in FFmpeg, but perhaps it
could be a "quick fix" until the MPlayer developers wake up?

Mats

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


Especially since MPlayer is such an evident use case of FFmpeg. It even
includes it inside its own source code.

Mats



For the record, at line 423 in libmpdemux/demux_lavf.c in MPlayer, it 
uses whatever is in extradata to tack onto the end of its "fake" 
BITMAPINFOHEADER, and which is later used to add this "extraneous" 
palette side data packet:


if(codec->extradata_size)
  memcpy(sh_video->bih + 1, codec->extradata, codec->extradata_siz

Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Write correct creation time of new ASF

[Nicolas George]

Thanks for the patch.

Le septidi 7 nivôse, an CCXXIV, Vadim Belov a écrit :
> -file_time = 0;
> +file_time = time(NULL);

Exposing the system time unconditionally has been rejected in the past due
to security considerations.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Carl Eugen Hoyos]

Mats Peterson  ffmpeg.org> writes:

> +memcpy(st->codec->extradata, matroska->palette,
> +AVPALETTE_SIZE);

As said, please remove this, you must not fix MPlayer 
issues in FFmpeg.
(The issue in MPlayer does not exist but that doesn't 
matter on this mailing list.)

Carl Eugen

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

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 03:16 AM, Michael Niedermayer wrote:

On Mon, Dec 28, 2015 at 12:24:10AM +, Carl Eugen Hoyos wrote:

Mats Peterson  ffmpeg.org> writes:


+memcpy(st->codec->extradata, matroska->palette,
+AVPALETTE_SIZE);





As said, please remove this,



you must not fix MPlayer
issues in FFmpeg.


this is true but iam not sure if this is really "just" a mplayer
issue.
If there is a global palette, then it is not unreasonable that it
is placed in a stream global place. This is especially true if not
all packets are copied and the palette from the first packet is
lost as a result.
either way, the commit message for this part just points at mplayer,
which isnt really good for a description of a "bug fix" in libavformat

Patch splited in move and matroska part
i removed this memcpy() for now from what i commited as there is
clearly no consenus on it.
Michael, the concensus is that MPlayer won't work without having the 
palette in extradata, since it uses the extradata as the palette when it 
adds ANOTHER palette side data packet AFTER the one added in 
matroskadec.c. I'm not making this up. Carl is of course right in his 
statement that we shouldn't fix MPlayer issues in FFmpeg, but perhaps it 
could be a "quick fix" until the MPlayer developers wake up?


Mats

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

Re: [FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 04:10 AM, Carl Eugen Hoyos wrote:

Michael Niedermayer  niedermayer.cc> writes:


Patch splited in move and matroska part
i removed this memcpy() for now from what i
commited as there is clearly no consenus on it


I would really have appreciated a real review:
Apart from the unrelated audio fix a significant
part of the patch was written by me.

And I suspect remuxing does not work yet, but
I can only test next week.

Carl Eugen

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



Well, if what you mean by "remuxing" is remuxing from QuickTime to 
Matroska, it has always been rather broken, but it doesn't concern me a 
lot at the moment since I'm using mkvmerge to remux.


Mats

--
Mats Peterson
http://matsp888.no-ip.org/~mats/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavu/libm: change macros to functions

[wm4]

On Thu, 24 Dec 2015 19:52:21 +0100
Hendrik Leppkes  wrote:

> Am 24.12.2015 19:34 schrieb "Ganesh Ajjanagadde" :
> >
> > In the standard library, these are functions. We should match it; there
> > is no reason for these to be macros.
> >
> > While at it, add some trivial comments for readability and correct an
> > incorrect (at standard double precision) M_LN2 constant used in the exp2
> > fallback.
> >
> > Signed-off-by: Ganesh Ajjanagadde 
> > ---
> >  libavutil/libm.h | 108  
> ++-
> >  1 file changed, 67 insertions(+), 41 deletions(-)
> >
> > diff --git a/libavutil/libm.h b/libavutil/libm.h
> > index 6f9ac1b..ac05613 100644
> > --- a/libavutil/libm.h
> > +++ b/libavutil/libm.h
> > @@ -36,33 +36,39 @@
> >  #endif /* HAVE_MIPSFPU && HAVE_INLINE_ASM*/
> >
> >  #if !HAVE_ATANF
> > -#undef atanf
> > -#define atanf(x) ((float)atan(x))
> > -#endif
> > +static av_always_inline float atanf(float x)
> > +{
> > +return atan(x);
> > +}
> > +#endif /* HAVE_ATANF */
> >
> >  #if !HAVE_ATAN2F
> > -#undef atan2f
> > -#define atan2f(y, x) ((float)atan2(y, x))
> > -#endif
> > +static av_always_inline float atan2f(float y, float x)
> > +{
> > +return atan2(y, x);
> > +}
> > +#endif /* HAVE_ATAN2F */
> >
> >  #if !HAVE_POWF
> > -#undef powf
> > -#define powf(x, y) ((float)pow(x, y))
> > -#endif
> > +static av_always_inline float powf(float x, float y)
> > +{
> > +return pow(x, y);
> > +}
> > +#endif /* HAVE_POWF */
> >
> >  #if !HAVE_CBRT
> >  static av_always_inline double cbrt(double x)
> >  {
> >  return x < 0 ? -pow(-x, 1.0 / 3.0) : pow(x, 1.0 / 3.0);
> >  }
> > -#endif
> > +#endif /* HAVE_CBRT */
> >
> >  #if !HAVE_CBRTF
> >  static av_always_inline float cbrtf(float x)
> >  {
> >  return x < 0 ? -powf(-x, 1.0 / 3.0) : powf(x, 1.0 / 3.0);
> >  }
> > -#endif
> > +#endif /* HAVE_CBRTF */
> >
> >  #if !HAVE_COPYSIGN
> >  static av_always_inline double copysign(double x, double y)
> > @@ -71,12 +77,13 @@ static av_always_inline double copysign(double x,  
> double y)
> >  uint64_t vy = av_double2int(y);
> >  return av_int2double((vx & UINT64_C(0x7fff)) | (vy &  
> UINT64_C(0x8000)));
> >  }
> > -#endif
> > +#endif /* HAVE_COPYSIGN */
> >
> >  #if !HAVE_COSF
> > -#undef cosf
> > -#define cosf(x) ((float)cos(x))
> > -#endif
> > +static av_always_inline float cosf(float x) {
> > +return cos(x);
> > +}
> > +#endif /* HAVE_COSF */
> >
> >  #if !HAVE_ERF
> >  static inline double ff_eval_poly(const double *coeff, int size, double  
> x) {
> > @@ -279,18 +286,24 @@ static inline double erf(double z)
> >  #endif
> >
> >  #if !HAVE_EXPF
> > -#undef expf
> > -#define expf(x) ((float)exp(x))
> > -#endif
> > +static av_always_inline float expf(float x)
> > +{
> > +return exp(x);
> > +}
> > +#endif /* HAVE_EXPF */
> >
> >  #if !HAVE_EXP2
> > -#undef exp2
> > -#define exp2(x) exp((x) * 0.693147180559945)
> > +static av_always_inline double exp2(double x)
> > +{
> > +return exp(x * M_LN2);
> > +}
> >  #endif /* HAVE_EXP2 */
> >
> >  #if !HAVE_EXP2F
> > -#undef exp2f
> > -#define exp2f(x) ((float)exp2(x))
> > +static av_always_inline float exp2f(float x)
> > +{
> > +return exp2(x);
> > +}
> >  #endif /* HAVE_EXP2F */
> >
> >  /* Somewhat inaccurate fallbacks, relative error ~ 1e-13 concentrated on  
> very
> > @@ -362,7 +375,6 @@ static av_always_inline av_const int  
> avpriv_isnan(double x)
> >  #endif /* HAVE_ISNAN */
> >
> >  #if !HAVE_HYPOT
> > -#undef hypot
> >  static inline av_const double hypot(double x, double y)
> >  {
> >  double ret, temp;
> > @@ -385,39 +397,53 @@ static inline av_const double hypot(double x,  
> double y)
> >  #endif /* HAVE_HYPOT */
> >
> >  #if !HAVE_LDEXPF
> > -#undef ldexpf
> > -#define ldexpf(x, exp) ((float)ldexp(x, exp))
> > -#endif
> > +static av_always_inline float ldexpf(float x, int exp)
> > +{
> > +return ldexp(x, exp);
> > +}
> > +#endif /* HAVE_LDEXPF */
> >
> >  #if !HAVE_LLRINT
> > -#undef llrint
> > -#define llrint(x) ((long long)rint(x))
> > +static av_always_inline long long llrint(double x)
> > +{
> > +return rint(x);
> > +}
> >  #endif /* HAVE_LLRINT */
> >
> >  #if !HAVE_LLRINTF
> > -#undef llrintf
> > -#define llrintf(x) ((long long)rint(x))
> > -#endif /* HAVE_LLRINT */
> > +static av_always_inline long long llrintf(float x)
> > +{
> > +return rint(x);
> > +}
> > +#endif /* HAVE_LLRINTF */
> >
> >  #if !HAVE_LOG2
> > -#undef log2
> > -#define log2(x) (log(x) * 1.44269504088896340736)
> > +static av_always_inline double log2(double x)
> > +{
> > +return log(x * 1.44269504088896340736);
> > +}
> >  #endif /* HAVE_LOG2 */
> >
> >  #if !HAVE_LOG2F
> > -#undef log2f
> > -#define log2f(x) ((float)log2(x))
> > +static av_always_inline float log2f(float x)
> > +{
> > +return log2(x);
> > +}
> >  #endif /* HAVE_LOG2F */
> >
> >  #if !HAVE_LOG10F
> > -#undef log10f
> > -#define