Re: [FFmpeg-devel] [PATCH]Remove probesize32 and max_analyze_duration32 on version bump

2015-09-04 Thread Carl Eugen Hoyos
James Almer  gmail.com> writes:

> Isn't removing these two going to break 
> compatibility with libav?

(ABI or API?)
I don't think so but I absolutely may miss something.

Carl Eugen

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


Re: [FFmpeg-devel] [PATCH][RFC] libavformat/mov Extend metadata handling to read in the keys from the 'keys' atom

2015-09-04 Thread Kevin Wheatley
On Thu, Sep 3, 2015 at 7:49 PM, Michael Niedermayer  wrote:
> missing checks for interger overflows of the addition and subtraction
>
> also the subject says "RFC", is there a reason not to push this to
> git master once it otherwise looks good ?


it is incomplete, basically I was fishing for a general OK to (ab)use
the dictionary in the MOVContext as a means to pass on the list of
keys, having done that, I've moved on to extend the data types
understood to include other data types (The sample ARRI clips need
signed and unsigned integers). I have this working but the code is in
need of a refactoring to say the least.

There does not appear to be a way to indicate the 'type' of data
pushed into the dictionary so that if I wanted to serialise it back
out I could get the same format, is this something people have
considered?

I could use a type dictionary to store the original type of the keys
along side the standard metatdata dictionary, but how that would feed
into the metatdata mux/demuxing and the command line interface I don't
know.

Thanks

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


[FFmpeg-devel] [PATCH] libavutil/dict: extend the list of convienience functions for storing different data types

2015-09-04 Thread Kevin Wheatley
Hi,

as part of adding support for non-string data types to .mov metadata,
I wondered about adding the following helper functions for storing
numeric types into an AVDictionary.

upfront I'll say I'm not 100% happy with the float32 and float64 named
variants (vs float and double) as there is no clear preference in
other areas of the code that I could see.

I'm also conscious that to provide for rewriting the .mov metadata
types, I'll need something different to store the actual types and
that might mean these functions would end up not being used at all.

Kevin
From 34fedb67d58402b519a7c91bff7623469802c4c4 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley 
Date: Fri, 4 Sep 2015 14:26:49 +0100
Subject: [PATCH] libavutil/dict: extend the list of convienience functions for storing different data types


Signed-off-by: Kevin Wheatley 
---
 libavutil/dict.c|   81 +-
 libavutil/dict.h|   24 +++
 tests/ref/fate/dict |   17 +++
 3 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 6ff1af5..4eaaf83 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -149,6 +149,33 @@ int av_dict_set_int(AVDictionary **pm, const char *key, int64_t value,
 return av_dict_set(pm, key, valuestr, flags);
 }
 
+int av_dict_set_uint(AVDictionary **pm, const char *key, uint64_t value,
+int flags)
+{
+char valuestr[22];
+snprintf(valuestr, sizeof(valuestr), "%"PRIu64, value);
+flags &= ~AV_DICT_DONT_STRDUP_VAL;
+return av_dict_set(pm, key, valuestr, flags);
+}
+
+int av_dict_set_float32(AVDictionary **pm, const char *key, float value,
+int flags)
+{
+char valuestr[17]; // sign + 1 + point + 8 + e + sign + 3 + term
+snprintf(valuestr, sizeof(valuestr), "%.9g", value);
+flags &= ~AV_DICT_DONT_STRDUP_VAL;
+return av_dict_set(pm, key, valuestr, flags);
+}
+
+int av_dict_set_float64(AVDictionary **pm, const char *key, double value,
+int flags)
+{
+char valuestr[26]; // sign + 1 + point + 16 + e + sign + 4 + term
+snprintf(valuestr, sizeof(valuestr), "%.17g", value);
+flags &= ~AV_DICT_DONT_STRDUP_VAL;
+return av_dict_set(pm, key, valuestr, flags);
+}
+
 static int parse_key_value_pair(AVDictionary **pm, const char **buf,
 const char *key_val_sep, const char *pairs_sep,
 int flags)
@@ -276,9 +303,13 @@ static void test_separators(const AVDictionary *m, const char pair, const char v
 int main(void)
 {
 AVDictionary *dict = NULL;
-AVDictionaryEntry *e;
+AVDictionaryEntry *e, *e2;
 char *buffer = NULL;
-
+float f32 = 0.0f;
+double f64 = 0.0f;
+union { uint32_t i; float f; } intfloat;
+union { uint64_t ui; int64_t i; } un_signed_int;
+
 printf("Testing av_dict_get_string() and av_dict_parse_string()\n");
 av_dict_get_string(dict, &buffer, '=', ',');
 printf("%s\n", buffer);
@@ -356,6 +387,52 @@ int main(void)
 printf("%s\n", e->value);
 av_dict_free(&dict);
 
+printf("\nTesting av_dict_set_int()\n");
+un_signed_int.ui = 0U;
+av_dict_set_int(&dict, "0", un_signed_int.i, 0);
+un_signed_int.ui = 0x8000UL;
+av_dict_set_int(&dict, "-max", un_signed_int.i, 0);
+un_signed_int.ui = 0x7fffUL;
+av_dict_set_int(&dict, "max", un_signed_int.i, 0);
+e = NULL;
+while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
+printf("%s %s\n", e->key, e->value);
+av_dict_free(&dict);
+
+printf("\nTesting av_dict_set_uint()\n");
+av_dict_set_uint(&dict, "0", 0, 0);
+av_dict_set_uint(&dict, "max", 0xUL, 0);
+e = NULL;
+while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
+printf("%s %s\n", e->key, e->value);
+av_dict_free(&dict);
+
+printf("\nTesting av_dict_set_float*()\n");
+// float and double representation of a given number should give the same value
+f32 = -1.20786635e-05f;
+f64 = -1.20786635e-05;
+av_dict_set_float32(&dict, "f32", f32, 0);
+e = av_dict_get(dict, "f32", NULL, 0);
+printf("%.30g => %s\n", f32, e->value);
+av_dict_set_float64(&dict, "f64", f64, 0);
+e2 = av_dict_get(dict, "f64", NULL, 0);
+printf("%.30g => %s\n", f64, e2->value);
+printf("%s == %s %s\n", e->value, e2->value, strcmp(e->value, e2->value) == 0 ? "OK" : "BAD");
+
+intfloat.i = 0xa647609;
+av_dict_set_float32(&dict, "0xa647609", intfloat.f, 0);
+e = av_dict_get(dict, "0xa647609", NULL, 0);
+printf("%.30g => %s\n", intfloat.f, e->value);
+
+// Next available bit pattern should not match
+++intfloat.i;
+av_dict_set_float32(&dict, "0xa64760a", intfloat.f, 0);
+e2 = av_dict_get(dict, "0xa64760a", NULL, 0);
+printf("%.30g => %s\n", intfloat.f, e2->value);
+
+printf("%s =! %s %s\n", e->value, e2->value, strcmp(e->value, e2->value) != 0 ?

Re: [FFmpeg-devel] [PATCH] libavutil/dict: extend the list of convienience functions for storing different data types

2015-09-04 Thread Nicolas George
L'octidi 18 fructidor, an CCXXIII, Kevin Wheatley a écrit :
> as part of adding support for non-string data types to .mov metadata,
> I wondered about adding the following helper functions for storing
> numeric types into an AVDictionary.
> 
> upfront I'll say I'm not 100% happy with the float32 and float64 named
> variants (vs float and double) as there is no clear preference in
> other areas of the code that I could see.

These function deal with CPU-ready values, not with the values serialized as
octets in memory; therefore, I am in favour of using the type names: float /
double. There is no guarantee that float is 32 bits and double 64. On
x86_32, double is sometimes 64 sometimes 80 depending on the FPU used.

> From 34fedb67d58402b519a7c91bff7623469802c4c4 Mon Sep 17 00:00:00 2001
> From: Kevin Wheatley 
> Date: Fri, 4 Sep 2015 14:26:49 +0100
> Subject: [PATCH] libavutil/dict: extend the list of convienience functions 
> for storing different data types
> 
> 
> Signed-off-by: Kevin Wheatley 
> ---
>  libavutil/dict.c|   81 +-
>  libavutil/dict.h|   24 +++
>  tests/ref/fate/dict |   17 +++
>  3 files changed, 120 insertions(+), 2 deletions(-)
> 
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index 6ff1af5..4eaaf83 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -149,6 +149,33 @@ int av_dict_set_int(AVDictionary **pm, const char *key, 
> int64_t value,
>  return av_dict_set(pm, key, valuestr, flags);
>  }
>  
> +int av_dict_set_uint(AVDictionary **pm, const char *key, uint64_t value,
> +int flags)
> +{
> +char valuestr[22];
> +snprintf(valuestr, sizeof(valuestr), "%"PRIu64, value);

> +flags &= ~AV_DICT_DONT_STRDUP_VAL;

I do not like that, it feels like defensive programming. API users should
not give random flags to functions and expect them to work. Therefore, I
would rather have the documentation state "AV_DICT_DONT_STRDUP_VAL must not
be set", implying that setting it is an undefined behaviour.

(And just to be nice, av_assert0() that it is not set: failing an assert is
the epitome of undefined behaviour, but nicer to debug than memory
corruption.)

> +return av_dict_set(pm, key, valuestr, flags);
> +}
> +
> +int av_dict_set_float32(AVDictionary **pm, const char *key, float value,
> +int flags)
> +{
> +char valuestr[17]; // sign + 1 + point + 8 + e + sign + 3 + term
> +snprintf(valuestr, sizeof(valuestr), "%.9g", value);
> +flags &= ~AV_DICT_DONT_STRDUP_VAL;
> +return av_dict_set(pm, key, valuestr, flags);
> +}
> +
> +int av_dict_set_float64(AVDictionary **pm, const char *key, double value,
> +int flags)
> +{
> +char valuestr[26]; // sign + 1 + point + 16 + e + sign + 4 + term
> +snprintf(valuestr, sizeof(valuestr), "%.17g", value);
> +flags &= ~AV_DICT_DONT_STRDUP_VAL;
> +return av_dict_set(pm, key, valuestr, flags);
> +}
> +
>  static int parse_key_value_pair(AVDictionary **pm, const char **buf,
>  const char *key_val_sep, const char 
> *pairs_sep,
>  int flags)
> @@ -276,9 +303,13 @@ static void test_separators(const AVDictionary *m, const 
> char pair, const char v
>  int main(void)
>  {
>  AVDictionary *dict = NULL;
> -AVDictionaryEntry *e;
> +AVDictionaryEntry *e, *e2;
>  char *buffer = NULL;
> -
> +float f32 = 0.0f;
> +double f64 = 0.0f;

> +union { uint32_t i; float f; } intfloat;
> +union { uint64_t ui; int64_t i; } un_signed_int;

What you are doing with these unions is highly non-portable and should be
avoided in the current code IMHO.

> +
>  printf("Testing av_dict_get_string() and av_dict_parse_string()\n");
>  av_dict_get_string(dict, &buffer, '=', ',');
>  printf("%s\n", buffer);
> @@ -356,6 +387,52 @@ int main(void)
>  printf("%s\n", e->value);
>  av_dict_free(&dict);
>  
> +printf("\nTesting av_dict_set_int()\n");
> +un_signed_int.ui = 0U;
> +av_dict_set_int(&dict, "0", un_signed_int.i, 0);
> +un_signed_int.ui = 0x8000UL;
> +av_dict_set_int(&dict, "-max", un_signed_int.i, 0);
> +un_signed_int.ui = 0x7fffUL;
> +av_dict_set_int(&dict, "max", un_signed_int.i, 0);
> +e = NULL;
> +while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
> +printf("%s %s\n", e->key, e->value);
> +av_dict_free(&dict);
> +
> +printf("\nTesting av_dict_set_uint()\n");
> +av_dict_set_uint(&dict, "0", 0, 0);
> +av_dict_set_uint(&dict, "max", 0xUL, 0);
> +e = NULL;
> +while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
> +printf("%s %s\n", e->key, e->value);
> +av_dict_free(&dict);
> +
> +printf("\nTesting av_dict_set_float*()\n");
> +// float and double representation of a given number should give the 
> same value
> +f32 = -1.20786635e-05f;
> +f64 =

[FFmpeg-devel] [PATCH] avfilter/vf_thumbnail: use the name 's' for the pointer to the private context

2015-09-04 Thread Ganesh Ajjanagadde
Signed-off-by: Ganesh Ajjanagadde 
---
 libavfilter/vf_thumbnail.c | 58 +++---
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/libavfilter/vf_thumbnail.c b/libavfilter/vf_thumbnail.c
index d70d063..c378791 100644
--- a/libavfilter/vf_thumbnail.c
+++ b/libavfilter/vf_thumbnail.c
@@ -58,15 +58,15 @@ AVFILTER_DEFINE_CLASS(thumbnail);
 
 static av_cold int init(AVFilterContext *ctx)
 {
-ThumbContext *thumb = ctx->priv;
+ThumbContext *s = ctx->priv;
 
-thumb->frames = av_calloc(thumb->n_frames, sizeof(*thumb->frames));
-if (!thumb->frames) {
+s->frames = av_calloc(s->n_frames, sizeof(*s->frames));
+if (!s->frames) {
 av_log(ctx, AV_LOG_ERROR,
"Allocation failure, try to lower the number of frames\n");
 return AVERROR(ENOMEM);
 }
-av_log(ctx, AV_LOG_VERBOSE, "batch size: %d frames\n", thumb->n_frames);
+av_log(ctx, AV_LOG_VERBOSE, "batch size: %d frames\n", s->n_frames);
 return 0;
 }
 
@@ -91,39 +91,39 @@ static double frame_sum_square_err(const int *hist, const 
double *median)
 static AVFrame *get_best_frame(AVFilterContext *ctx)
 {
 AVFrame *picref;
-ThumbContext *thumb = ctx->priv;
+ThumbContext *s = ctx->priv;
 int i, j, best_frame_idx = 0;
-int nb_frames = thumb->n;
+int nb_frames = s->n;
 double avg_hist[HIST_SIZE] = {0}, sq_err, min_sq_err = -1;
 
 // average histogram of the N frames
 for (j = 0; j < FF_ARRAY_ELEMS(avg_hist); j++) {
 for (i = 0; i < nb_frames; i++)
-avg_hist[j] += (double)thumb->frames[i].histogram[j];
+avg_hist[j] += (double)s->frames[i].histogram[j];
 avg_hist[j] /= nb_frames;
 }
 
 // find the frame closer to the average using the sum of squared errors
 for (i = 0; i < nb_frames; i++) {
-sq_err = frame_sum_square_err(thumb->frames[i].histogram, avg_hist);
+sq_err = frame_sum_square_err(s->frames[i].histogram, avg_hist);
 if (i == 0 || sq_err < min_sq_err)
 best_frame_idx = i, min_sq_err = sq_err;
 }
 
 // free and reset everything (except the best frame buffer)
 for (i = 0; i < nb_frames; i++) {
-memset(thumb->frames[i].histogram, 0, 
sizeof(thumb->frames[i].histogram));
+memset(s->frames[i].histogram, 0, sizeof(s->frames[i].histogram));
 if (i != best_frame_idx)
-av_frame_free(&thumb->frames[i].buf);
+av_frame_free(&s->frames[i].buf);
 }
-thumb->n = 0;
+s->n = 0;
 
 // raise the chosen one
-picref = thumb->frames[best_frame_idx].buf;
+picref = s->frames[best_frame_idx].buf;
 av_log(ctx, AV_LOG_INFO, "frame id #%d (pts_time=%f) selected "
"from a set of %d images\n", best_frame_idx,
-   picref->pts * av_q2d(thumb->tb), nb_frames);
-thumb->frames[best_frame_idx].buf = NULL;
+   picref->pts * av_q2d(s->tb), nb_frames);
+s->frames[best_frame_idx].buf = NULL;
 
 return picref;
 }
@@ -132,13 +132,13 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
*frame)
 {
 int i, j;
 AVFilterContext *ctx  = inlink->dst;
-ThumbContext *thumb   = ctx->priv;
+ThumbContext *s   = ctx->priv;
 AVFilterLink *outlink = ctx->outputs[0];
-int *hist = thumb->frames[thumb->n].histogram;
+int *hist = s->frames[s->n].histogram;
 const uint8_t *p = frame->data[0];
 
 // keep a reference of each frame
-thumb->frames[thumb->n].buf = frame;
+s->frames[s->n].buf = frame;
 
 // update current frame RGB histogram
 for (j = 0; j < inlink->h; j++) {
@@ -151,8 +151,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
*frame)
 }
 
 // no selection until the buffer of N frames is filled up
-thumb->n++;
-if (thumb->n < thumb->n_frames)
+s->n++;
+if (s->n < s->n_frames)
 return 0;
 
 return ff_filter_frame(outlink, get_best_frame(ctx));
@@ -161,22 +161,22 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
*frame)
 static av_cold void uninit(AVFilterContext *ctx)
 {
 int i;
-ThumbContext *thumb = ctx->priv;
-for (i = 0; i < thumb->n_frames && thumb->frames[i].buf; i++)
-av_frame_free(&thumb->frames[i].buf);
-av_freep(&thumb->frames);
+ThumbContext *s = ctx->priv;
+for (i = 0; i < s->n_frames && s->frames[i].buf; i++)
+av_frame_free(&s->frames[i].buf);
+av_freep(&s->frames);
 }
 
 static int request_frame(AVFilterLink *link)
 {
 AVFilterContext *ctx = link->src;
-ThumbContext *thumb = ctx->priv;
+ThumbContext *s = ctx->priv;
 
 /* loop until a frame thumbnail is available (when a frame is queued,
- * thumb->n is reset to zero) */
+ * s->n is reset to zero) */
 do {
 int ret = ff_request_frame(ctx->inputs[0]);
-if (ret == AVERROR_EOF && thumb->n) {
+if (ret == AVERROR_EOF && s->n) {
 ret = ff_filter_frame(link, get_best_frame(ctx));
  

Re: [FFmpeg-devel] [PATCH] libavutil/dict: extend the list of convienience functions for storing different data types

2015-09-04 Thread wm4
On Fri, 4 Sep 2015 14:38:54 +0100
Kevin Wheatley  wrote:

> Hi,
> 
> as part of adding support for non-string data types to .mov metadata,
> I wondered about adding the following helper functions for storing
> numeric types into an AVDictionary.
> 
> upfront I'll say I'm not 100% happy with the float32 and float64 named
> variants (vs float and double) as there is no clear preference in
> other areas of the code that I could see.

I don't understand it either. Why not just use float and double,
instead of obfuscating the names further? (It'd be a difference if this
function e.g. serialized the floats to binary - but it literally just
passes them as-is to libc. Having the C data type in the argument seems
advantageous.

Also, what's the use in having a float function at all?

I'd call av_dict_set_uint av_dict_set_uint64.

> I'm also conscious that to provide for rewriting the .mov metadata
> types, I'll need something different to store the actual types and
> that might mean these functions would end up not being used at all.
> 
> Kevin

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


Re: [FFmpeg-devel] [PATCH] libavutil/dict: extend the list of convienience functions for storing different data types

2015-09-04 Thread Kevin Wheatley
On Fri, Sep 4, 2015 at 4:18 PM, wm4  wrote:
> On Fri, 4 Sep 2015 14:38:54 +0100
> Kevin Wheatley  wrote:
>
>> Hi,
>>
>> as part of adding support for non-string data types to .mov metadata,
>> I wondered about adding the following helper functions for storing
>> numeric types into an AVDictionary.
>>
>> upfront I'll say I'm not 100% happy with the float32 and float64 named
>> variants (vs float and double) as there is no clear preference in
>> other areas of the code that I could see.
>
> I don't understand it either. Why not just use float and double,
> instead of obfuscating the names further? (It'd be a difference if this
> function e.g. serialized the floats to binary - but it literally just
> passes them as-is to libc. Having the C data type in the argument seems
> advantageous.
>
> Also, what's the use in having a float function at all?
>
> I'd call av_dict_set_uint av_dict_set_uint64.

I'll explain my POV...

Essentially these helpers do the same as the existing
av_dict_set_int() does for int64_t so I've essentially just copied
that and substituted the required conversions from  to char
array. The floating point is needed to correctly decode those formats.

Using the bit length in the name of the floating point variation of
the functions is because they will only correctly produce the textual
representation sufficient to transform back into the same binary
representation based on the bit lengths of the floating point
representation (they assume IEEE single and double precision in the
lengths of the formatting options and thus buffer size).

Mentally I was in the middle of the Apple QuickTime metadata
representations where they also use the floatXX terminology - not a
great excuse I know :-)


Re Nicholas' comments on the

flags &= ~AV_DICT_DONT_STRDUP_VAL;

I was simply replicating the existing form of the function av_dict_set_int().

As to portability in the tests - yes I agree - happy for somebody to
point me towards a good solution for that.


My alternative to these would have been to embed the formatting in the
libavformat/mov.c code which obviously would have less impact.

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


Re: [FFmpeg-devel] [PATCH]Remove probesize32 and max_analyze_duration32 on version bump

2015-09-04 Thread James Almer
On 9/4/2015 5:53 AM, Carl Eugen Hoyos wrote:
> James Almer  gmail.com> writes:
> 
>> Isn't removing these two going to break 
>> compatibility with libav?
> 
> (ABI or API?)
> I don't think so but I absolutely may miss something.
> 
> Carl Eugen

ABI, you're removing elements from the middle of the struct.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: Make MP3 seek fast

2015-09-04 Thread Tsung-Hung Wu
Hi FFmpeg developers,

Sorry for the spam. I know you are busy. Please take your time to review
the patch. I don't mean to rush at all.
Since the is my first patch, I just want to make sure I was doing the right
thing. Please kindly let me know if I posted to a wrong mail group or made
the patch wrong. Thank you so much for all your great contributions.

Best Regards,
Andy


On Tue, Sep 1, 2015 at 11:35 AM, Tsung-Hung Wu 
wrote:

> Hi FFmpeg developers,
>
> I am trying to solve an issue: seeking a large MP3 file is slow. This
> happens when play Podcast audio, for example
>
> http://podcastdownload.npr.org/anon.npr-podcasts/podcast/344098539/430765119/npr_430765119.mp3
> It works not so good in less computing power mobile devices, especially
> under slow network environment.
> My goal is to improve the usability, although it may sacrifice the
> accuracy.
>
> I noticed that FFmpeg has AVFMT_FLAG_FAST_SEEK flag. That's exactly what I
> need, sacrifices the accuracy of playback time in exchange for
> responsiveness. However, it does not work as my expectation. I made some
> changes to fulfill my needs. Please find the attachment for the patch.
> Just to be clear, I put some comments below for discussions, which are not
> in the patch.
>
> Could you please kindly review the attached patch? Let me know if you have
> any comments. Thank you so much for your help.
>
> Best Regards,
> Andy
>
> ---
>  libavformat/mp3dec.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index 007c6ea..7f675ab 100644
> --- a/libavformat/mp3dec.c
> +++ b/libavformat/mp3dec.c
> @@ -342,7 +342,7 @@ static int mp3_read_header(AVFormatContext *s)
>  int i;
>
>  if (mp3->usetoc < 0)
> -mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 0 : 2;
> +mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
>
> /*
> Should usetoc be 0 when AVFMT_FLAG_FAST_SEEK is enabled? When usetoc is 0,
> seeking a VBR mp3 is slow, even if it contains a TOC.
> Thus, I think 1 is a more proper selection.
> */
>
>  st = avformat_new_stream(s, NULL);
>  if (!st)
> @@ -489,19 +489,21 @@ static int mp3_seek(AVFormatContext *s, int
> stream_index, int64_t timestamp,
>  AVStream *st = s->streams[0];
>  int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
>  int64_t best_pos;
> +int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
> +int64_t filesize = mp3->header_filesize > 0 ? mp3->header_filesize
> +: avio_size(s->pb) - s->internal->data_offset;
>
> /*
> Many MP3 files in Podcast does not have VBRI, LAME, or XING tag, so
> header_filesize is not always available. (file size - data offset) could be
> a good estimation.
> */
>
>
>  if (mp3->usetoc == 2)
>  return -1; // generic index code
>
> -if (   mp3->is_cbr
> +if (   (mp3->is_cbr || fast_seek)
>  && (mp3->usetoc == 0 || !mp3->xing_toc)
>  && st->duration > 0
> -&& mp3->header_filesize > s->internal->data_offset
> -&& mp3->frames) {
> +&& filesize > 0) {
> /*
> Not sure why (mp3->header_filesize > s->internal->data_offset) has to be
> true. What if a MP3 file contains a short audio and a large picture?
> Again, mp3->frames is not always available. Do we really need it here? I
> move the check to below.
> */
>  ie = &ie1;
>  timestamp = av_clip64(timestamp, 0, st->duration);
>  ie->timestamp = timestamp;
> -ie->pos   = av_rescale(timestamp, mp3->header_filesize,
> st->duration) + s->internal->data_offset;
> +ie->pos   = av_rescale(timestamp, filesize, st->duration) +
> s->internal->data_offset;
>  } else if (mp3->xing_toc) {
>  if (ret < 0)
>  return ret;
> @@ -515,7 +517,7 @@ static int mp3_seek(AVFormatContext *s, int
> stream_index, int64_t timestamp,
>  if (best_pos < 0)
>  return best_pos;
>
> -if (mp3->is_cbr && ie == &ie1) {
> +if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
>  int frame_duration = av_rescale(st->duration, 1, mp3->frames);
>  ie1.timestamp = frame_duration * av_rescale(best_pos -
> s->internal->data_offset, mp3->frames, mp3->header_filesize);
>  }
> --
> 2.5.0.457.gab17608
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: Make MP3 seek fast

2015-09-04 Thread Paul B Mahol
On 9/4/15, Tsung-Hung Wu  wrote:
> Hi FFmpeg developers,
>
> Sorry for the spam. I know you are busy. Please take your time to review
> the patch. I don't mean to rush at all.
> Since the is my first patch, I just want to make sure I was doing the right
> thing. Please kindly let me know if I posted to a wrong mail group or made
> the patch wrong. Thank you so much for all your great contributions.
>
> Best Regards,
> Andy
>

First do not top post, second you already received review.

>
> On Tue, Sep 1, 2015 at 11:35 AM, Tsung-Hung Wu 
> wrote:
>
>> Hi FFmpeg developers,
>>
>> I am trying to solve an issue: seeking a large MP3 file is slow. This
>> happens when play Podcast audio, for example
>>
>> http://podcastdownload.npr.org/anon.npr-podcasts/podcast/344098539/430765119/npr_430765119.mp3
>> It works not so good in less computing power mobile devices, especially
>> under slow network environment.
>> My goal is to improve the usability, although it may sacrifice the
>> accuracy.
>>
>> I noticed that FFmpeg has AVFMT_FLAG_FAST_SEEK flag. That's exactly what I
>> need, sacrifices the accuracy of playback time in exchange for
>> responsiveness. However, it does not work as my expectation. I made some
>> changes to fulfill my needs. Please find the attachment for the patch.
>> Just to be clear, I put some comments below for discussions, which are not
>> in the patch.
>>
>> Could you please kindly review the attached patch? Let me know if you have
>> any comments. Thank you so much for your help.
>>
>> Best Regards,
>> Andy
>>
>> ---
>>  libavformat/mp3dec.c | 14 --
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>> index 007c6ea..7f675ab 100644
>> --- a/libavformat/mp3dec.c
>> +++ b/libavformat/mp3dec.c
>> @@ -342,7 +342,7 @@ static int mp3_read_header(AVFormatContext *s)
>>  int i;
>>
>>  if (mp3->usetoc < 0)
>> -mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 0 : 2;
>> +mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
>>
>> /*
>> Should usetoc be 0 when AVFMT_FLAG_FAST_SEEK is enabled? When usetoc is 0,
>> seeking a VBR mp3 is slow, even if it contains a TOC.
>> Thus, I think 1 is a more proper selection.
>> */
>>
>>  st = avformat_new_stream(s, NULL);
>>  if (!st)
>> @@ -489,19 +489,21 @@ static int mp3_seek(AVFormatContext *s, int
>> stream_index, int64_t timestamp,
>>  AVStream *st = s->streams[0];
>>  int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
>>  int64_t best_pos;
>> +int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
>> +int64_t filesize = mp3->header_filesize > 0 ? mp3->header_filesize
>> +: avio_size(s->pb) -
>> s->internal->data_offset;
>>
>> /*
>> Many MP3 files in Podcast does not have VBRI, LAME, or XING tag, so
>> header_filesize is not always available. (file size - data offset) could
>> be
>> a good estimation.
>> */
>>
>>
>>  if (mp3->usetoc == 2)
>>  return -1; // generic index code
>>
>> -if (   mp3->is_cbr
>> +if (   (mp3->is_cbr || fast_seek)
>>  && (mp3->usetoc == 0 || !mp3->xing_toc)
>>  && st->duration > 0
>> -&& mp3->header_filesize > s->internal->data_offset
>> -&& mp3->frames) {
>> +&& filesize > 0) {
>> /*
>> Not sure why (mp3->header_filesize > s->internal->data_offset) has to be
>> true. What if a MP3 file contains a short audio and a large picture?
>> Again, mp3->frames is not always available. Do we really need it here? I
>> move the check to below.
>> */
>>  ie = &ie1;
>>  timestamp = av_clip64(timestamp, 0, st->duration);
>>  ie->timestamp = timestamp;
>> -ie->pos   = av_rescale(timestamp, mp3->header_filesize,
>> st->duration) + s->internal->data_offset;
>> +ie->pos   = av_rescale(timestamp, filesize, st->duration) +
>> s->internal->data_offset;
>>  } else if (mp3->xing_toc) {
>>  if (ret < 0)
>>  return ret;
>> @@ -515,7 +517,7 @@ static int mp3_seek(AVFormatContext *s, int
>> stream_index, int64_t timestamp,
>>  if (best_pos < 0)
>>  return best_pos;
>>
>> -if (mp3->is_cbr && ie == &ie1) {
>> +if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
>>  int frame_duration = av_rescale(st->duration, 1, mp3->frames);
>>  ie1.timestamp = frame_duration * av_rescale(best_pos -
>> s->internal->data_offset, mp3->frames, mp3->header_filesize);
>>  }
>> --
>> 2.5.0.457.gab17608
>>
>>
> ___
> 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] vp9: always sync segmentation.feat between threads.

2015-09-04 Thread Ronald S. Bultje
---
 libavcodec/vp9.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 238185a..e6c5389 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -4340,10 +4340,8 @@ static int 
vp9_decode_update_thread_context(AVCodecContext *dst, const AVCodecCo
 s->bpp_index = ssrc->bpp_index;
 memcpy(&s->prob_ctx, &ssrc->prob_ctx, sizeof(s->prob_ctx));
 memcpy(&s->lf_delta, &ssrc->lf_delta, sizeof(s->lf_delta));
-if (ssrc->segmentation.enabled) {
-memcpy(&s->segmentation.feat, &ssrc->segmentation.feat,
-   sizeof(s->segmentation.feat));
-}
+memcpy(&s->segmentation.feat, &ssrc->segmentation.feat,
+   sizeof(s->segmentation.feat));
 
 return 0;
 }
-- 
2.1.2

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


[FFmpeg-devel] [PATCH] vp9: sync segmentation.absolute_vals between threads.

2015-09-04 Thread Ronald S. Bultje
---
 libavcodec/vp9.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index e6c5389..3bd2a0f 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -4335,6 +4335,7 @@ static int 
vp9_decode_update_thread_context(AVCodecContext *dst, const AVCodecCo
 s->ss_h = ssrc->ss_h;
 s->segmentation.enabled = ssrc->segmentation.enabled;
 s->segmentation.update_map = ssrc->segmentation.update_map;
+s->segmentation.absolute_vals = ssrc->segmentation.absolute_vals;
 s->bytesperpixel = ssrc->bytesperpixel;
 s->bpp = ssrc->bpp;
 s->bpp_index = ssrc->bpp_index;
-- 
2.1.2

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


[FFmpeg-devel] [PATCH] vp9: do unscaled MC in scaled path if size of this reference matches.

2015-09-04 Thread Ronald S. Bultje
This can happen if we do bidirectional MC, where one reference has the
same size as the current frame, but the other one doesn't.
---
 libavcodec/vp9.c | 219 +--
 1 file changed, 117 insertions(+), 102 deletions(-)

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 3bd2a0f..25a7b1d 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -2766,7 +2766,108 @@ static void intra_recon_16bpp(AVCodecContext *ctx, 
ptrdiff_t y_off, ptrdiff_t uv
 intra_recon(ctx, y_off, uv_off, 2);
 }
 
+static av_always_inline void mc_luma_unscaled(VP9Context *s, vp9_mc_func 
(*mc)[2],
+  uint8_t *dst, ptrdiff_t 
dst_stride,
+  const uint8_t *ref, ptrdiff_t 
ref_stride,
+  ThreadFrame *ref_frame,
+  ptrdiff_t y, ptrdiff_t x, const 
VP56mv *mv,
+  int bw, int bh, int w, int h, 
int bytesperpixel)
+{
+int mx = mv->x, my = mv->y, th;
+
+y += my >> 3;
+x += mx >> 3;
+ref += y * ref_stride + x * bytesperpixel;
+mx &= 7;
+my &= 7;
+// FIXME bilinear filter only needs 0/1 pixels, not 3/4
+// we use +7 because the last 7 pixels of each sbrow can be changed in
+// the longest loopfilter of the next sbrow
+th = (y + bh + 4 * !!my + 7) >> 6;
+ff_thread_await_progress(ref_frame, FFMAX(th, 0), 0);
+if (x < !!mx * 3 || y < !!my * 3 ||
+x + !!mx * 4 > w - bw || y + !!my * 4 > h - bh) {
+s->vdsp.emulated_edge_mc(s->edge_emu_buffer,
+ ref - !!my * 3 * ref_stride - !!mx * 3 * 
bytesperpixel,
+ 160, ref_stride,
+ bw + !!mx * 7, bh + !!my * 7,
+ x - !!mx * 3, y - !!my * 3, w, h);
+ref = s->edge_emu_buffer + !!my * 3 * 160 + !!mx * 3 * bytesperpixel;
+ref_stride = 160;
+}
+mc[!!mx][!!my](dst, dst_stride, ref, ref_stride, bh, mx << 1, my << 1);
+}
+
+static av_always_inline void mc_chroma_unscaled(VP9Context *s, vp9_mc_func 
(*mc)[2],
+uint8_t *dst_u, uint8_t *dst_v,
+ptrdiff_t dst_stride,
+const uint8_t *ref_u, 
ptrdiff_t src_stride_u,
+const uint8_t *ref_v, 
ptrdiff_t src_stride_v,
+ThreadFrame *ref_frame,
+ptrdiff_t y, ptrdiff_t x, 
const VP56mv *mv,
+int bw, int bh, int w, int h, 
int bytesperpixel)
+{
+int mx = mv->x << !s->ss_h, my = mv->y << !s->ss_v, th;
+
+y += my >> 4;
+x += mx >> 4;
+ref_u += y * src_stride_u + x * bytesperpixel;
+ref_v += y * src_stride_v + x * bytesperpixel;
+mx &= 15;
+my &= 15;
+// FIXME bilinear filter only needs 0/1 pixels, not 3/4
+// we use +7 because the last 7 pixels of each sbrow can be changed in
+// the longest loopfilter of the next sbrow
+th = (y + bh + 4 * !!my + 7) >> (6 - s->ss_v);
+ff_thread_await_progress(ref_frame, FFMAX(th, 0), 0);
+if (x < !!mx * 3 || y < !!my * 3 ||
+x + !!mx * 4 > w - bw || y + !!my * 4 > h - bh) {
+s->vdsp.emulated_edge_mc(s->edge_emu_buffer,
+ ref_u - !!my * 3 * src_stride_u - !!mx * 3 * 
bytesperpixel,
+ 160, src_stride_u,
+ bw + !!mx * 7, bh + !!my * 7,
+ x - !!mx * 3, y - !!my * 3, w, h);
+ref_u = s->edge_emu_buffer + !!my * 3 * 160 + !!mx * 3 * bytesperpixel;
+mc[!!mx][!!my](dst_u, dst_stride, ref_u, 160, bh, mx, my);
+
+s->vdsp.emulated_edge_mc(s->edge_emu_buffer,
+ ref_v - !!my * 3 * src_stride_v - !!mx * 3 * 
bytesperpixel,
+ 160, src_stride_v,
+ bw + !!mx * 7, bh + !!my * 7,
+ x - !!mx * 3, y - !!my * 3, w, h);
+ref_v = s->edge_emu_buffer + !!my * 3 * 160 + !!mx * 3 * bytesperpixel;
+mc[!!mx][!!my](dst_v, dst_stride, ref_v, 160, bh, mx, my);
+} else {
+mc[!!mx][!!my](dst_u, dst_stride, ref_u, src_stride_u, bh, mx, my);
+mc[!!mx][!!my](dst_v, dst_stride, ref_v, src_stride_v, bh, mx, my);
+}
+}
+
+#define mc_luma_dir(s, mc, dst, dst_ls, src, src_ls, tref, row, col, mv, \
+px, py, pw, ph, bw, bh, w, h, i) \
+mc_luma_unscaled(s, s->dsp.mc, dst, dst_ls, src, src_ls, tref, row, col, \
+ mv, bw, bh, w, h, bytesperpixel)
+#define mc_chroma_dir(s, mc, dstu, dstv, dst_ls, srcu, srcu_ls, srcv, srcv_ls, 
tref, \
+  row, col,

[FFmpeg-devel] [PATCH] vp9: fix edge copy for 10/12bpp frames.

2015-09-04 Thread Ronald S. Bultje
---
 libavcodec/vp9.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 25a7b1d..7624743 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -3353,9 +3353,9 @@ static void decode_b(AVCodecContext *ctx, int row, int 
col,
 
 av_assert2(n <= 4);
 if (w & bw) {
-s->dsp.mc[n][0][0][0][0](f->data[0] + yoff + o, f->linesize[0],
- s->tmp_y + o, 128, h, 0, 0);
-o += bw * bytesperpixel;
+s->dsp.mc[n][0][0][0][0](f->data[0] + yoff + o * 
bytesperpixel, f->linesize[0],
+ s->tmp_y + o * bytesperpixel, 128, h, 
0, 0);
+o += bw;
 }
 }
 }
@@ -3368,11 +3368,11 @@ static void decode_b(AVCodecContext *ctx, int row, int 
col,
 
 av_assert2(n <= 4);
 if (w & bw) {
-s->dsp.mc[n][0][0][0][0](f->data[1] + uvoff + o, 
f->linesize[1],
- s->tmp_uv[0] + o, 128, h, 0, 0);
-s->dsp.mc[n][0][0][0][0](f->data[2] + uvoff + o, 
f->linesize[2],
- s->tmp_uv[1] + o, 128, h, 0, 0);
-o += bw * bytesperpixel;
+s->dsp.mc[n][0][0][0][0](f->data[1] + uvoff + o * 
bytesperpixel, f->linesize[1],
+ s->tmp_uv[0] + o * bytesperpixel, 
128, h, 0, 0);
+s->dsp.mc[n][0][0][0][0](f->data[2] + uvoff + o * 
bytesperpixel, f->linesize[2],
+ s->tmp_uv[1] + o * bytesperpixel, 
128, h, 0, 0);
+o += bw;
 }
 }
 }
-- 
2.1.2

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


[FFmpeg-devel] [PATCH] avformat/mp3dec: Make MP3 seek fast

2015-09-04 Thread Tsung-Hung Wu
Thanks wm4. I updated the patch according to your comments.


>* @@ -489,19 +489,21 @@ static int mp3_seek(AVFormatContext *s, int
*>* stream_index, int64_t timestamp,
*>*  AVStream *st = s->streams[0];
*>*  int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
*>*  int64_t best_pos;
*>* +int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
*>* +int64_t filesize = mp3->header_filesize > 0 ? mp3->header_filesize
*>* +: avio_size(s->pb) - s->internal->data_offset;
*
> ok. But maybe the avio_size() return value should be checked. It can
> return an error.

Done. Please see the patch. Thanks for your comments.

> >* /*
*>* Many MP3 files in Podcast does not have VBRI, LAME, or XING tag, so
*>* header_filesize is not always available. (file size - data offset) could be
*>* a good estimation.
*>* */
*> > >*  if (mp3->usetoc == 2)
*>*  return -1; // generic index code
*> >* -if (   mp3->is_cbr
*>* +if (   (mp3->is_cbr || fast_seek)
*
> Not sure why you need this. If the file is VBR and has no TOC, then all
> odds are off. But I guess this is ok on the AVFMT_FLAG_FAST_SEEK code
> path, if you think it's more useful this way.

Yes, I really need the seek operation fast, and AVFMT_FLAG_FAST_SEEK
allows to sacrifice the accuracy in exchange for responsiveness. For
VBR files without TOC, like you said, it's not accurate. For CBR files
without INFO tag, it's fast and reasonable accurate. Since
AVFMT_FLAG_FAST_SEEK is set, we should make it fast instead of make it
safe.

>*  && (mp3->usetoc == 0 || !mp3->xing_toc)
*>*  && st->duration > 0
*>* -&& mp3->header_filesize > s->internal->data_offset
*>* -&& mp3->frames) {
*>* +&& filesize > 0) {
*>* /*
*>* Not sure why (mp3->header_filesize > s->internal->data_offset) has to be
*>* true. What if a MP3 file contains a short audio and a large picture?
*>* Again, mp3->frames is not always available. Do we really need it here? I
*>* move the check to below.
*>* */
*
> I can't think of any reason either. Maybe the original intention of
> this code was actually to check whether header_size is a sane value at
> all? There are lots of weird corner cases, like incomplete or
> concatenated mp3 files.

>*  ie = &ie1;
*>*  timestamp = av_clip64(timestamp, 0, st->duration);
*>*  ie->timestamp = timestamp;
*>* -ie->pos   = av_rescale(timestamp, mp3->header_filesize,
*>* st->duration) + s->internal->data_offset;
*>* +ie->pos   = av_rescale(timestamp, filesize, st->duration) +
*>* s->internal->data_offset;
*>*  } else if (mp3->xing_toc) {
*>*  if (ret < 0)
*>*  return ret;
*>* @@ -515,7 +517,7 @@ static int mp3_seek(AVFormatContext *s, int
*>* stream_index, int64_t timestamp,
*>*  if (best_pos < 0)
*>*  return best_pos;
*> >* -if (mp3->is_cbr && ie == &ie1) {
*>* +if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
*
> Not sure what this does after all.

This tries to refine the timestamp. Because it may seek to middle of a
frame, mp3_sync() will align the position to a frame boundary. Thus,
the timestamp may be a little bit off.

Since it's the only place using mp3->frames in mp3_seek()*, *I think
we can check it right before using it.

>*  int frame_duration = av_rescale(st->duration, 1, mp3->frames);
*>*  ie1.timestamp = frame_duration * av_rescale(best_pos -
*>* s->internal->data_offset, mp3->frames, mp3->header_filesize);
*>*  }*
From 094c6efab6c5eec1fec274bf1bcace1987ae7d03 Mon Sep 17 00:00:00 2001
From: Andy Wu 
Date: Mon, 31 Aug 2015 17:08:30 -0700
Subject: [PATCH] avformat/mp3dec: Make MP3 seek fast

When AVFMT_FLAG_FAST_SEEK is specified, make MP3 seek operation as
fast as possible.

When no "-usetoc" is specified, the default operation is using TOC
if available; otherwise, uses linear interpolation. This is useful
when seeking a large MP3 file with no TOC available. One example is
Podcast, many MP3 files are large, but no CBR/VBR tags. Most of
them are actually CBR. Even in VBR cases, this option sacrifices the
accuracy of playback time in exchange for responsiveness.
---
 libavformat/mp3dec.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 007c6ea..d3080d7 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -342,7 +342,7 @@ static int mp3_read_header(AVFormatContext *s)
 int i;
 
 if (mp3->usetoc < 0)
-mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 0 : 2;
+mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
 
 st = avformat_new_stream(s, NULL);
 if (!st)
@@ -489,19 +489,26 @@ static int mp3_seek(AVFormatContext *s, int stream_index, int64_t timestamp,
 AVStream *st = s->streams[0];
 int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
 int64_t best_pos;
+int fast_seek = (s->flags & AVF

Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: Make MP3 seek fast

2015-09-04 Thread Tsung-Hung Wu
From 094c6efab6c5eec1fec274bf1bcace1987ae7d03 Mon Sep 17 00:00:00 2001
From: Andy Wu 
Date: Mon, 31 Aug 2015 17:08:30 -0700
Subject: [PATCH] avformat/mp3dec: Make MP3 seek fast

When AVFMT_FLAG_FAST_SEEK is specified, make MP3 seek operation as
fast as possible.

When no "-usetoc" is specified, the default operation is using TOC
if available; otherwise, uses linear interpolation. This is useful
when seeking a large MP3 file with no TOC available. One example is
Podcast, many MP3 files are large, but no CBR/VBR tags. Most of
them are actually CBR. Even in VBR cases, this option sacrifices the
accuracy of playback time in exchange for responsiveness.
---
 libavformat/mp3dec.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 007c6ea..d3080d7 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -342,7 +342,7 @@ static int mp3_read_header(AVFormatContext *s)
 int i;

 if (mp3->usetoc < 0)
-mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 0 : 2;
+mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;

 st = avformat_new_stream(s, NULL);
 if (!st)
@@ -489,19 +489,26 @@ static int mp3_seek(AVFormatContext *s, int
stream_index, int64_t timestamp,
 AVStream *st = s->streams[0];
 int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
 int64_t best_pos;
+int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
+int64_t filesize = mp3->header_filesize;

 if (mp3->usetoc == 2)
 return -1; // generic index code

-if (   mp3->is_cbr
+if (filesize <= 0) {
+int64_t size = avio_size(s->pb);
+if (size > 0 && size > s->internal->data_offset)
+filesize = size - s->internal->data_offset;
+}
+
+if (   (mp3->is_cbr || fast_seek)
 && (mp3->usetoc == 0 || !mp3->xing_toc)
 && st->duration > 0
-&& mp3->header_filesize > s->internal->data_offset
-&& mp3->frames) {
+&& filesize > 0) {
 ie = &ie1;
 timestamp = av_clip64(timestamp, 0, st->duration);
 ie->timestamp = timestamp;
-ie->pos   = av_rescale(timestamp, mp3->header_filesize,
st->duration) + s->internal->data_offset;
+ie->pos   = av_rescale(timestamp, filesize, st->duration) +
s->internal->data_offset;
 } else if (mp3->xing_toc) {
 if (ret < 0)
 return ret;
@@ -515,7 +522,7 @@ static int mp3_seek(AVFormatContext *s, int
stream_index, int64_t timestamp,
 if (best_pos < 0)
 return best_pos;

-if (mp3->is_cbr && ie == &ie1) {
+if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
 int frame_duration = av_rescale(st->duration, 1, mp3->frames);
 ie1.timestamp = frame_duration * av_rescale(best_pos -
s->internal->data_offset, mp3->frames, mp3->header_filesize);
 }
-- 
2.5.0.457.gab17608
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] vp9: fix integer overflow in 10/12bpp DC-only calculation.

2015-09-04 Thread Ronald S. Bultje
---
 libavcodec/vp9dsp_template.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vp9dsp_template.c b/libavcodec/vp9dsp_template.c
index 5a8578a..9395a0c 100644
--- a/libavcodec/vp9dsp_template.c
+++ b/libavcodec/vp9dsp_template.c
@@ -1131,8 +1131,8 @@ static void 
type_a##_##type_b##_##sz##x##sz##_add_c(uint8_t *_dst, \
 \
 stride /= sizeof(pixel); \
 if (has_dconly && eob == 1) { \
-const int t  = (((block[0] * 11585 + (1 << 13)) >> 14) \
-   * 11585 + (1 << 13)) >> 14; \
+const int t  = dctint) block[0] * 11585 + (1 << 13)) >> 14) \
+* 11585 + (1 << 13)) >> 14; \
 block[0] = 0; \
 for (i = 0; i < sz; i++) { \
 for (j = 0; j < sz; j++) \
-- 
2.1.2

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


[FFmpeg-devel] [PATCH] vp9: fix type of iadst4_1d intermediates.

2015-09-04 Thread Ronald S. Bultje
Fixes integer overflows for extreme coefficient values in 10/12bpp content.
---
 libavcodec/vp9dsp_template.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vp9dsp_template.c b/libavcodec/vp9dsp_template.c
index 9395a0c..4d810fe 100644
--- a/libavcodec/vp9dsp_template.c
+++ b/libavcodec/vp9dsp_template.c
@@ -1186,7 +1186,7 @@ static av_always_inline void idct4_1d(const dctcoef *in, 
ptrdiff_t stride,
 static av_always_inline void iadst4_1d(const dctcoef *in, ptrdiff_t stride,
dctcoef *out, int pass)
 {
-int t0, t1, t2, t3;
+dctint t0, t1, t2, t3;
 
 t0 =  5283 * IN(0) + 15212 * IN(2) +  9929 * IN(3);
 t1 =  9929 * IN(0) -  5283 * IN(2) - 15212 * IN(3);
-- 
2.1.2

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


Re: [FFmpeg-devel] [PATCH]Remove probesize32 and max_analyze_duration32 on version bump

2015-09-04 Thread Carl Eugen Hoyos
James Almer  gmail.com> writes:
> >> Isn't removing these two going to break 
> >> compatibility with libav?
> > 
> > (ABI or API?)
> > I don't think so but I absolutely may miss something.

> ABI, you're removing elements from the middle of the 
> struct.

In which situation would that be an issue?

Carl Eugen

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


Re: [FFmpeg-devel] [PATCH]Remove probesize32 and max_analyze_duration32 on version bump

2015-09-04 Thread James Almer
On 9/4/2015 6:19 PM, Carl Eugen Hoyos wrote:
> James Almer  gmail.com> writes:
 Isn't removing these two going to break 
 compatibility with libav?
>>>
>>> (ABI or API?)
>>> I don't think so but I absolutely may miss something.
> 
>> ABI, you're removing elements from the middle of the 
>> struct.
> 
> In which situation would that be an issue?
> 
> Carl Eugen

Figured that since direct access is apparently allowed for elements
above ts_id it would be an issue for applications built with libav
that then use ffmpeg's lavf at runtime.

Disregard this if that's not the case.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: Make MP3 seek fast

2015-09-04 Thread wm4
On Fri, 4 Sep 2015 13:55:22 -0700
Tsung-Hung Wu  wrote:

> From 094c6efab6c5eec1fec274bf1bcace1987ae7d03 Mon Sep 17 00:00:00 2001
> From: Andy Wu 
> Date: Mon, 31 Aug 2015 17:08:30 -0700
> Subject: [PATCH] avformat/mp3dec: Make MP3 seek fast
> 
> When AVFMT_FLAG_FAST_SEEK is specified, make MP3 seek operation as
> fast as possible.
> 
> When no "-usetoc" is specified, the default operation is using TOC
> if available; otherwise, uses linear interpolation. This is useful
> when seeking a large MP3 file with no TOC available. One example is
> Podcast, many MP3 files are large, but no CBR/VBR tags. Most of
> them are actually CBR. Even in VBR cases, this option sacrifices the
> accuracy of playback time in exchange for responsiveness.
> ---
>  libavformat/mp3dec.c | 19 +--
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index 007c6ea..d3080d7 100644
> --- a/libavformat/mp3dec.c
> +++ b/libavformat/mp3dec.c
> @@ -342,7 +342,7 @@ static int mp3_read_header(AVFormatContext *s)
>  int i;
> 
>  if (mp3->usetoc < 0)
> -mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 0 : 2;
> +mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
> 
>  st = avformat_new_stream(s, NULL);
>  if (!st)
> @@ -489,19 +489,26 @@ static int mp3_seek(AVFormatContext *s, int
> stream_index, int64_t timestamp,
>  AVStream *st = s->streams[0];
>  int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
>  int64_t best_pos;
> +int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
> +int64_t filesize = mp3->header_filesize;
> 
>  if (mp3->usetoc == 2)
>  return -1; // generic index code
> 
> -if (   mp3->is_cbr
> +if (filesize <= 0) {
> +int64_t size = avio_size(s->pb);
> +if (size > 0 && size > s->internal->data_offset)
> +filesize = size - s->internal->data_offset;
> +}
> +
> +if (   (mp3->is_cbr || fast_seek)
>  && (mp3->usetoc == 0 || !mp3->xing_toc)
>  && st->duration > 0
> -&& mp3->header_filesize > s->internal->data_offset
> -&& mp3->frames) {
> +&& filesize > 0) {
>  ie = &ie1;
>  timestamp = av_clip64(timestamp, 0, st->duration);
>  ie->timestamp = timestamp;
> -ie->pos   = av_rescale(timestamp, mp3->header_filesize,
> st->duration) + s->internal->data_offset;
> +ie->pos   = av_rescale(timestamp, filesize, st->duration) +
> s->internal->data_offset;
>  } else if (mp3->xing_toc) {
>  if (ret < 0)
>  return ret;
> @@ -515,7 +522,7 @@ static int mp3_seek(AVFormatContext *s, int
> stream_index, int64_t timestamp,
>  if (best_pos < 0)
>  return best_pos;
> 
> -if (mp3->is_cbr && ie == &ie1) {
> +if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
>  int frame_duration = av_rescale(st->duration, 1, mp3->frames);
>  ie1.timestamp = frame_duration * av_rescale(best_pos -
> s->internal->data_offset, mp3->frames, mp3->header_filesize);
>  }

Patch seems ok. I'll run FATE and apply it tomorrow, unless someone
else has comments. Thanks for the patch.

You've sent it twice; not sure if that was intended? I've looked at the
second one for the purpose of the review. (They're probably the same.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]Remove probesize32 and max_analyze_duration32 on version bump

2015-09-04 Thread Hendrik Leppkes
On Fri, Sep 4, 2015 at 11:43 PM, James Almer  wrote:
> On 9/4/2015 6:19 PM, Carl Eugen Hoyos wrote:
>> James Almer  gmail.com> writes:
> Isn't removing these two going to break
> compatibility with libav?

 (ABI or API?)
 I don't think so but I absolutely may miss something.
>>
>>> ABI, you're removing elements from the middle of the
>>> struct.
>>
>> In which situation would that be an issue?
>>
>> Carl Eugen
>
> Figured that since direct access is apparently allowed for elements
> above ts_id it would be an issue for applications built with libav
> that then use ffmpeg's lavf at runtime.
>
> Disregard this if that's not the case.

We are not really ABI compatible, and its not a goal worth going after.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]Support more mxf files with codec_ul

2015-09-04 Thread Carl Eugen Hoyos
Carl Eugen Hoyos  ag.or.at> writes:

> Please also look at the patch attached in ticket #4820, 
> I will add the additional uls if you don't object.

I pushed these additional codec_uls since they affect 
decoding only together with my patch posted here.

Hope that's ok, Carl Eugen

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


Re: [FFmpeg-devel] [PATCH]Remove experimental flag from j2k encoder

2015-09-04 Thread Carl Eugen Hoyos
Carl Eugen Hoyos  ag.or.at> writes:

>  .init   = j2kenc_init,
>  .encode2= encode_frame,
>  .close  = j2kenc_destroy,
> -.capabilities   = AV_CODEC_CAP_EXPERIMENTAL,

Patch applied.

Carl Eugen

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


Re: [FFmpeg-devel] [PATCH]Remove probesize32 and max_analyze_duration32 on version bump

2015-09-04 Thread Carl Eugen Hoyos
Hendrik Leppkes  gmail.com> writes:

> We are not really ABI compatible, and its not a goal 
> worth going after.

I thought so too.

Carl Eugen

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


Re: [FFmpeg-devel] [PATCH]Remove probesize32 and max_analyze_duration32 on version bump

2015-09-04 Thread James Almer
On 9/4/2015 7:06 PM, Hendrik Leppkes wrote:
> On Fri, Sep 4, 2015 at 11:43 PM, James Almer  wrote:
>> On 9/4/2015 6:19 PM, Carl Eugen Hoyos wrote:
>>> James Almer  gmail.com> writes:
>> Isn't removing these two going to break
>> compatibility with libav?
>
> (ABI or API?)
> I don't think so but I absolutely may miss something.
>>>
 ABI, you're removing elements from the middle of the
 struct.
>>>
>>> In which situation would that be an issue?
>>>
>>> Carl Eugen
>>
>> Figured that since direct access is apparently allowed for elements
>> above ts_id it would be an issue for applications built with libav
>> that then use ffmpeg's lavf at runtime.
>>
>> Disregard this if that's not the case.
> 
> We are not really ABI compatible, and its not a goal worth going after.

Wonder why then is the code littered with duplicate defines and enum
values (for example fourcc-like values for AVCodecID), setter/getter
functions for struct elements not available in both projects, functions
with different signatures and elements in some structs with different
offsets depending if that incompatible_libav_abi option was requested
during configure, etc...

Some files are a mess purely because of compatibility considerations.
If in the end it's not a concern, then we should grab a broom after
bumping major versions this weekend.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]Remove probesize32 and max_analyze_duration32 on version bump

2015-09-04 Thread wm4
On Fri, 4 Sep 2015 19:42:18 -0300
James Almer  wrote:

> On 9/4/2015 7:06 PM, Hendrik Leppkes wrote:
> > On Fri, Sep 4, 2015 at 11:43 PM, James Almer  wrote:
> >> On 9/4/2015 6:19 PM, Carl Eugen Hoyos wrote:
> >>> James Almer  gmail.com> writes:
> >> Isn't removing these two going to break
> >> compatibility with libav?
> >
> > (ABI or API?)
> > I don't think so but I absolutely may miss something.
> >>>
>  ABI, you're removing elements from the middle of the
>  struct.
> >>>
> >>> In which situation would that be an issue?
> >>>
> >>> Carl Eugen
> >>
> >> Figured that since direct access is apparently allowed for elements
> >> above ts_id it would be an issue for applications built with libav
> >> that then use ffmpeg's lavf at runtime.
> >>
> >> Disregard this if that's not the case.
> > 
> > We are not really ABI compatible, and its not a goal worth going after.
> 
> Wonder why then is the code littered with duplicate defines and enum
> values (for example fourcc-like values for AVCodecID), setter/getter
> functions for struct elements not available in both projects, functions
> with different signatures and elements in some structs with different
> offsets depending if that incompatible_libav_abi option was requested
> during configure, etc...
> 
> Some files are a mess purely because of compatibility considerations.
> If in the end it's not a concern, then we should grab a broom after
> bumping major versions this weekend.

You know what, it's a perfect time to clean this up. I can remove the
Libav ABI compat hack and reassign the crazy FourCC enums, if such a
patch will get accepted. (We can change them because it's ABI bump
time.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] vp9: check return value of ff_thread_ref_frame().

2015-09-04 Thread Ronald S. Bultje
Fixes CID 1322309.
---
 libavcodec/vp9.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 7624743..e67c761 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -4250,7 +4250,8 @@ static int vp9_decode_frame(AVCodecContext *ctx, void 
*frame,
 for (i = 0; i < 8; i++) {
 if (s->refs[i].f->data[0])
 ff_thread_release_buffer(ctx, &s->refs[i]);
-ff_thread_ref_frame(&s->refs[i], &s->next_refs[i]);
+if ((res = ff_thread_ref_frame(&s->refs[i], &s->next_refs[i])) < 0)
+return res;
 }
 
 if (!s->invisible) {
-- 
2.1.2

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


Re: [FFmpeg-devel] [PATCH] vp9: check return value of ff_thread_ref_frame().

2015-09-04 Thread Michael Niedermayer
On Fri, Sep 04, 2015 at 07:33:29PM -0400, Ronald S. Bultje wrote:
> Fixes CID 1322309.
> ---
>  libavcodec/vp9.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

this breaks fate

--- ./tests/ref/fate/vp9-16-intra-only  2015-09-05 00:47:48.445308437 +0200
+++ tests/data/fate/vp9-16-intra-only   2015-09-05 01:48:55.637385695 +0200
@@ -3,10 +3,6 @@
 #hash: MD5
 #tb 0: 12/359
 #stream#, dts,pts, duration, size, hash
-0,  0,  0,1,   152064, d57529601178948afa4818c3c8938884
-0,  1,  1,1,   152064, d47e00250c45733d64af067a417bcd06
-0,  2,  2,1,   152064, 984e41cd8350808ac6129746b2377818
-0,  3,  3,1,   152064, a5fa62996b4bb52e72e335722cf55bef
-0,  4,  4,1,   152064, b71ca5ad650170ac921a71a6440fb508
-0,  5,  5,1,   152064, 76ba63001170b8992fc72be5c4ace731
-0,  6,  6,1,   152064, c4e7f96a8fd58d901b1d881926ddae09
+0,  1,  1,1,   152064, 984e41cd8350808ac6129746b2377818
+0,  2,  2,1,   152064, a5fa62996b4bb52e72e335722cf55bef
+0,  3,  3,1,   152064, b71ca5ad650170ac921a71a6440fb508
Test vp9-16-intra-only failed. Look at tests/data/fate/vp9-16-intra-only.err 
for details.


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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


[FFmpeg-devel] [PATCH] vp9: check return value of ff_thread_ref_frame().

2015-09-04 Thread Ronald S. Bultje
Fixes CID 1322309.
---
 libavcodec/vp9.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 7624743..25e7419 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -4250,7 +4250,9 @@ static int vp9_decode_frame(AVCodecContext *ctx, void 
*frame,
 for (i = 0; i < 8; i++) {
 if (s->refs[i].f->data[0])
 ff_thread_release_buffer(ctx, &s->refs[i]);
-ff_thread_ref_frame(&s->refs[i], &s->next_refs[i]);
+if (s->next_refs[i].f->data[0] &&
+(res = ff_thread_ref_frame(&s->refs[i], &s->next_refs[i])) < 0)
+return res;
 }
 
 if (!s->invisible) {
-- 
2.1.2

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


Re: [FFmpeg-devel] [PATCH] vp9: check return value of ff_thread_ref_frame().

2015-09-04 Thread Michael Niedermayer
On Fri, Sep 04, 2015 at 08:11:05PM -0400, Ronald S. Bultje wrote:
> Fixes CID 1322309.
> ---
>  libavcodec/vp9.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

LGTM

thanks

[...]
-- 
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