Re: [FFmpeg-devel] [PATCH]Remove probesize32 and max_analyze_duration32 on version bump
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
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
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
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
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
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
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
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
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
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.
--- 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.
--- 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.
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.
--- 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
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
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.
--- 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.
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
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
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
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
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
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
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
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
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
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().
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().
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().
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().
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