[FFmpeg-devel] [PATCH]libavformat/http: fix http error eof
Hi: I find an issue about http. I don't use chunked, so s->chunksize will be set as UINT64_MAX when http open, but because of "if (s->chunksize > 0) s->chunksize -= len;" then chunksize will not be UINT64_MAX. If ffurl_read return to 0, s->off < target_end, http_buf_read will return to 0, then this will lead to eof, so this is incorrect, and http_buf_read should return to AVERROR(EIO). 0001-libavformat-http-return-EIO-when-ffurl_read-return-0.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] lavc: add a framework to fix alignment problems.
A lot of codecs require aligned frame data, but do not document it. It can result in crashes with perfectly valid uses of the API. Design rationale: - requiring frame data to be always aligned would be wasteful; - alignment requirements will evolve (the 16->32 bump is still recent); - requiring applications to worry about alignment is not convenient. For now, the default alignment is fixed at 32. Fix: trac ticket #6349 Signed-off-by: Nicolas George --- libavcodec/avcodec.h | 10 + libavcodec/encode.c | 57 +--- libavcodec/utils.c | 2 ++ 3 files changed, 66 insertions(+), 3 deletions(-) Without the 1<<. diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 00f9c82afc..1e45cf28b2 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -3682,6 +3682,16 @@ typedef struct AVCodecContext { * (with the display dimensions being determined by the crop_* fields). */ int apply_cropping; + +/** + * Minimum alignment of frame data required by the codec. + * All frame data pointers must have the alignment lower bits cleared, + * i.e. be a multiple of alignment. + * - encoding: set by the encoder and used by the framework + * - decoding: unused + */ +unsigned alignment; + } AVCodecContext; AVRational av_codec_get_pkt_timebase (const AVCodecContext *avctx); diff --git a/libavcodec/encode.c b/libavcodec/encode.c index 525ee1f5d6..11d5b2f7c7 100644 --- a/libavcodec/encode.c +++ b/libavcodec/encode.c @@ -115,6 +115,40 @@ fail: return ret; } +static int frame_realign(AVCodecContext *avctx, const AVFrame **frame, AVFrame **realigned) +{ +AVFrame *tmp; +int ret; + +if (!*frame || av_frame_check_align(*frame, avctx->alignment)) +return 0; + +tmp = av_frame_alloc(); +if (!tmp) +return AVERROR(ENOMEM); +tmp->format = (*frame)->format; +tmp->width = (*frame)->width; +tmp->height = (*frame)->height; +tmp->channels = (*frame)->channels; +tmp->channel_layout = (*frame)->channel_layout; +tmp->nb_samples = (*frame)->nb_samples; +ret = av_frame_get_buffer(tmp, avctx->alignment); +if (ret < 0) +goto fail; +ret = av_frame_copy(tmp, *frame); +if (ret < 0) +return ret; +ret = av_frame_copy_props(tmp, *frame); +if (ret < 0) +return ret; +*frame = *realigned = tmp; +return 0; + +fail: +av_frame_free(&tmp); +return ret; +} + int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, AVPacket *avpkt, const AVFrame *frame, @@ -122,6 +156,7 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, { AVFrame *extended_frame = NULL; AVFrame *padded_frame = NULL; +AVFrame *realigned_frame = NULL; int ret; AVPacket user_pkt = *avpkt; int needs_realloc = !user_pkt.data; @@ -195,6 +230,9 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, av_assert0(avctx->codec->encode2); +ret = frame_realign(avctx, &frame, &realigned_frame); +if (ret < 0) +goto end; ret = avctx->codec->encode2(avctx, avpkt, frame, got_packet_ptr); if (!ret) { if (*got_packet_ptr) { @@ -252,6 +290,7 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, end: av_frame_free(&padded_frame); +av_frame_free(&realigned_frame); av_free(extended_frame); #if FF_API_AUDIOENC_DELAY @@ -269,6 +308,7 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx, int ret; AVPacket user_pkt = *avpkt; int needs_realloc = !user_pkt.data; +AVFrame *realigned_frame = NULL; *got_packet_ptr = 0; @@ -301,7 +341,9 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx, av_assert0(avctx->codec->encode2); -ret = avctx->codec->encode2(avctx, avpkt, frame, got_packet_ptr); +ret = frame_realign(avctx, &frame, &realigned_frame); +if (!ret) +ret = avctx->codec->encode2(avctx, avpkt, frame, got_packet_ptr); av_assert0(ret <= 0); emms_c(); @@ -340,6 +382,7 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx, avctx->frame_number++; } +av_frame_free(&realigned_frame); if (ret < 0 || !*got_packet_ptr) av_packet_unref(avpkt); @@ -406,8 +449,16 @@ int attribute_align_arg avcodec_send_frame(AVCodecContext *avctx, const AVFrame return 0; } -if (avctx->codec->send_frame) -return avctx->codec->send_frame(avctx, frame); +if (avctx->codec->send_frame) { +AVFrame *realigned_frame = NULL; +int ret; +ret = frame_realign(avctx, &frame, &realigned_frame); +if (ret < 0) +return ret; +ret = avctx->codec->send_frame(avctx, frame); +
[FFmpeg-devel] [PATCH 3/3] lavfi: add a framework to fix alignment problems.
A lot of filters require aligned frame data, but do not document it. It can result in crashes with perfectly valid uses of the API. For now, the default alignment is not set. Signed-off-by: Nicolas George --- libavfilter/avfilter.c | 51 ++ libavfilter/avfilter.h | 9 + 2 files changed, 60 insertions(+) diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index 08b86b010d..ff49e80289 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -166,6 +166,7 @@ int avfilter_link(AVFilterContext *src, unsigned srcpad, link->type= src->output_pads[srcpad].type; av_assert0(AV_PIX_FMT_NONE == -1 && AV_SAMPLE_FMT_NONE == -1); link->format = -1; +link->alignment = 1; ff_framequeue_init(&link->fifo, &src->graph->internal->frame_queues); return 0; @@ -1515,15 +1516,60 @@ static void consume_update(AVFilterLink *link, const AVFrame *frame) link->frame_count_out++; } +static int frame_realign(AVFilterLink *link, AVFrame *frame) +{ +AVFrame *tmp; +int ret; + +if (!frame || av_frame_check_align(frame, link->alignment)) +return 0; + +switch (link->type) { +case AVMEDIA_TYPE_VIDEO: +tmp = ff_get_video_buffer(link, link->w, link->h); +break; +case AVMEDIA_TYPE_AUDIO: +tmp = ff_get_audio_buffer(link, frame->nb_samples); +break; +default: +av_assert0(!"reached"); +} +if (!tmp) +return AVERROR(ENOMEM); + +if (ret < 0) +goto fail; +ret = av_frame_copy(tmp, frame); +if (ret < 0) +return ret; +ret = av_frame_copy_props(tmp, frame); +if (ret < 0) +return ret; +av_frame_unref(frame); +av_frame_move_ref(frame, tmp); +av_frame_free(&tmp); +return 0; + +fail: +av_frame_free(&tmp); +return ret; +} + int ff_inlink_consume_frame(AVFilterLink *link, AVFrame **rframe) { AVFrame *frame; +int ret; *rframe = NULL; if (!ff_inlink_check_available_frame(link)) return 0; frame = ff_framequeue_take(&link->fifo); consume_update(link, frame); +ret = frame_realign(link, frame); +if (ret < 0) { +av_frame_free(&frame); +return ret; +} *rframe = frame; return 1; } @@ -1544,6 +1590,11 @@ int ff_inlink_consume_samples(AVFilterLink *link, unsigned min, unsigned max, if (ret < 0) return ret; consume_update(link, frame); +ret = frame_realign(link, frame); +if (ret < 0) { +av_frame_free(&frame); +return ret; +} *rframe = frame; return 1; } diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h index 60662c19ac..16543a666e 100644 --- a/libavfilter/avfilter.h +++ b/libavfilter/avfilter.h @@ -569,6 +569,15 @@ struct AVFilterLink { */ AVBufferRef *hw_frames_ctx; +/** + * Minimum alignment of frame data required by the destination filter. + * All frame data pointers must have the alignment lower bits cleared, + * i.e. be a multiple of alignment. + * Frames with insufficient alignment will be realigned by the + * framework. + */ +unsigned alignment; + #ifndef FF_INTERNAL_FIELDS /** -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
Signed-off-by: Nicolas George --- doc/APIchanges| 3 +++ libavutil/frame.c | 17 + libavutil/frame.h | 8 3 files changed, 28 insertions(+) With the linesize check and without the 1<<. diff --git a/doc/APIchanges b/doc/APIchanges index 67a6142401..6d3b573c2d 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2015-08-28 API changes, most recent first: +2017-05-18 - xx - lavu 55.63.100 - frame.h + Add av_frame_check_align(). + 2017-05-15 - xx - lavc 57.96.100 - avcodec.h VideoToolbox hardware-accelerated decoding now supports the new hwaccel API, which can create the decoder context and allocate hardware frames automatically. diff --git a/libavutil/frame.c b/libavutil/frame.c index 24d5d5f184..aed3cd04ec 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -781,3 +781,20 @@ const char *av_frame_side_data_name(enum AVFrameSideDataType type) } return NULL; } + +int av_frame_check_align(const AVFrame *frame, unsigned align) +{ +unsigned mask = align - 1; +unsigned i; + +for (i = 0; i < AV_NUM_DATA_POINTERS; i++) +if (((intptr_t)frame->data[i] & mask) || +(frame->linesize[i] & mask)) +return 0; +if (!frame->extended_data || frame->extended_data == frame->data) +return 1; +for (i = AV_NUM_DATA_POINTERS; i < frame->channels; i++) +if (((intptr_t)frame->extended_data[i] & mask)) +return 0; +return 1; +} diff --git a/libavutil/frame.h b/libavutil/frame.h index 26261d7e40..1cbf7c7a5a 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -772,6 +772,14 @@ void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType type); const char *av_frame_side_data_name(enum AVFrameSideDataType type); /** + * Check if the data pointers of a frame are aligned enough. + * Test if all frame data pointers have the alignment lower bits cleared, + * i.e. are a multiple of alignment. + * @return >0 if aligned, 0 if not + */ +int av_frame_check_align(const AVFrame *frame, unsigned align); + +/** * @} */ -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
On Thu, May 18, 2017 at 10:11 AM, Nicolas George wrote: > Signed-off-by: Nicolas George > --- > doc/APIchanges| 3 +++ > libavutil/frame.c | 17 + > libavutil/frame.h | 8 > 3 files changed, 28 insertions(+) > > I wonder if there should be an exception in here somewhere for hardware pixelformats, there is no reason to assume the hardware pointers passed in AVFrame would be aligned, and aligning them would cause all sorts of crashes. Best case from an API standpoint would be if av_frame_check_align can just claim hardware frames are aligned, so users don't have to replicate that check. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : > I wonder if there should be an exception in here somewhere for > hardware pixelformats, there is no reason to assume the hardware > pointers passed in AVFrame would be aligned, and aligning them would > cause all sorts of crashes. > Best case from an API standpoint would be if av_frame_check_align can > just claim hardware frames are aligned, so users don't have to > replicate that check. Possible, but I think setting the correct value for alignment is the best way of dealing with this kind of issue. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
On Thu, May 18, 2017 at 10:44 AM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : >> I wonder if there should be an exception in here somewhere for >> hardware pixelformats, there is no reason to assume the hardware >> pointers passed in AVFrame would be aligned, and aligning them would >> cause all sorts of crashes. >> Best case from an API standpoint would be if av_frame_check_align can >> just claim hardware frames are aligned, so users don't have to >> replicate that check. > > Possible, but I think setting the correct value for alignment is the > best way of dealing with this kind of issue. > I think its a saner choice to design the API to try to avoid instant heap corruption, instead of hoping every case sets the alignment properly in all cases. A single if condition shouldn't be too much trouble, we already flag hardware pixel formats as such in the pixdesc to identify them. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : > I think its a saner choice to design the API to try to avoid instant > heap corruption, instead of hoping every case sets the alignment > properly in all cases. > A single if condition shouldn't be too much trouble, we already flag > hardware pixel formats as such in the pixdesc to identify them. Giving an incorrect alignment value to this function will not cause an "instant heap corruption", you are burning a straw man. This function performs a simple task: it checks the alignment of a frame, given as a parameter, against a required alignment value, given as a parameter. Simple, straightforward, predictable. Adding special cases to this function would break these properties. Also, I am roughly one bikeshedding mail away from losing interest in this whole matter. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
On Thu, May 18, 2017 at 3:11 PM, Nicolas George wrote: > Signed-off-by: Nicolas George > --- > doc/APIchanges| 3 +++ > libavutil/frame.c | 17 + > libavutil/frame.h | 8 > 3 files changed, 28 insertions(+) > > > With the linesize check and without the 1<<. > > > diff --git a/doc/APIchanges b/doc/APIchanges > index 67a6142401..6d3b573c2d 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,9 @@ libavutil: 2015-08-28 > > API changes, most recent first: > > +2017-05-18 - xx - lavu 55.63.100 - frame.h > + Add av_frame_check_align(). > + > 2017-05-15 - xx - lavc 57.96.100 - avcodec.h >VideoToolbox hardware-accelerated decoding now supports the new hwaccel > API, >which can create the decoder context and allocate hardware frames > automatically. > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 24d5d5f184..aed3cd04ec 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -781,3 +781,20 @@ const char *av_frame_side_data_name(enum > AVFrameSideDataType type) > } > return NULL; > } > + > +int av_frame_check_align(const AVFrame *frame, unsigned align) > +{ > +unsigned mask = align - 1; > +unsigned i; > + > +for (i = 0; i < AV_NUM_DATA_POINTERS; i++) > +if (((intptr_t)frame->data[i] & mask) || > +(frame->linesize[i] & mask)) > +return 0; > +if (!frame->extended_data || frame->extended_data == frame->data) > +return 1; > +for (i = AV_NUM_DATA_POINTERS; i < frame->channels; i++) > +if (((intptr_t)frame->extended_data[i] & mask)) > +return 0; Missing linesize check. > +return 1; > +} > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 26261d7e40..1cbf7c7a5a 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -772,6 +772,14 @@ void av_frame_remove_side_data(AVFrame *frame, enum > AVFrameSideDataType type); > const char *av_frame_side_data_name(enum AVFrameSideDataType type); > > /** > + * Check if the data pointers of a frame are aligned enough. > + * Test if all frame data pointers have the alignment lower bits cleared, > + * i.e. are a multiple of alignment. > + * @return >0 if aligned, 0 if not > + */ > +int av_frame_check_align(const AVFrame *frame, unsigned align); > + > +/** > * @} > */ > > -- > 2.11.0 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I tested the behaviour of av_frame_get_buffer(frame, 32) and check linesize[0], here the result of some different pixel formats: yuv420p: 32 rgb24: 96 rgba: 32 So linesize constraint acutually depends on pixel format. IMHO, because the fact of crop filter, actually (probably all) video filters and codecs accept unaligned data pointer but (some of them) require aligned linesize. Thank's. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : > Missing linesize check. No, check again. > I tested the behaviour of av_frame_get_buffer(frame, 32) and check > linesize[0], here the result of some different pixel formats: > yuv420p: 32 > rgb24: 96 > rgba: 32 > > So linesize constraint acutually depends on pixel format. No, check again. Regards, -- Nicola George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
On Thu, May 18, 2017 at 11:16 AM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : >> I think its a saner choice to design the API to try to avoid instant >> heap corruption, instead of hoping every case sets the alignment >> properly in all cases. >> A single if condition shouldn't be too much trouble, we already flag >> hardware pixel formats as such in the pixdesc to identify them. > > Giving an incorrect alignment value to this function will not cause an > "instant heap corruption", you are burning a straw man. If as a result of this function returning 0 some other code tries to memcpy some opaque hardware pointer, there is a good chance of something crazy happening, so its hardly a strawman. We could put the check in another central place, but IMHO it needs to be somewhere central, and not hope that every filter and encoder remembers to override alignment in the hardware surface input case. > > This function performs a simple task: it checks the alignment of a > frame, given as a parameter, against a required alignment value, given > as a parameter. Simple, straightforward, predictable. > > Adding special cases to this function would break these properties. This simple task relies on various properties of the input frame - which in the hardware case are just not given, so its essentially performing an undefined operation. > > Also, I am roughly one bikeshedding mail away from losing interest in > this whole matter. > This is not bikeshedding, its an actual concern. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : > We could put the check in another central place Yes. > This is not bikeshedding, its an actual concern. Well, you win, I drop the baby, catch it or not, I do not care. But I still oppose to bad fixed being committed in areas of code I am working on. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
On Thu, May 18, 2017 at 5:20 PM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >> Missing linesize check. > > No, check again. Oh, I'm sorry. My fault. > >> I tested the behaviour of av_frame_get_buffer(frame, 32) and check >> linesize[0], here the result of some different pixel formats: >> yuv420p: 32 >> rgb24: 96 >> rgba: 32 >> >> So linesize constraint acutually depends on pixel format. > > No, check again. I mean for rgb24 format, when alignment is 32, linesize should be multiple of 96 (32 is not enough). Thank's. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] New to the list... thinking about a feature
Hi folks, I'm completely new to the list but have a couple of years experience as a user. I happen to work a lot with things I record from satellite. My recorder will include a lot of empty streams in the transport stream and that is annoying. I have tweaked ffmpeg here and there to show only non-empty streams at the 'normal' verbosity level and include empty streams at heightened verbosity. Is that of interest? Wanting to contribute, /PA -- Fragen sind nicht da um beantwortet zu werden, Fragen sind da um gestellet zu werden Georg Kreisler ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : > I mean for rgb24 format, when alignment is 32, linesize should be > multiple of 96 (32 is not enough). I think you are wrong. I do not know any alignment requirement that is not a power of 2. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
On Thu, May 18, 2017 at 5:30 PM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >> I mean for rgb24 format, when alignment is 32, linesize should be >> multiple of 96 (32 is not enough). > > I think you are wrong. I do not know any alignment requirement that is > not a power of 2. But that is what av_frame_get_buffer does (and maybe ff_get_buffer too). Thank's. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().
Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : > But that is what av_frame_get_buffer does (and maybe ff_get_buffer too). Maybe it does that, but I do not think it is an alignment issue, and I am not entirely convinced this is intentional. Anyway, I have wasted enough of my time on this issue. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: Enable in MP4 container both AMR-NB and AMR-WB
Hey Carl, I am not aware of non-FFmpeg apps which can record to MP4 containing AMR. That's not to say one does not exist. I would agree that we likely need no changes for playing these MP4 files. The MP4 requirement was from one of our customers. Thanks, Bob On Wed, May 17, 2017 at 8:01 PM, Carl Eugen Hoyos wrote: > 2017-05-17 23:26 GMT+02:00 Bob Kirnum : > > Did not realize the files I shared were too large. I copied them to a > > shared Google Drive here . . . > > The question was if you have files that were not created with FFmpeg. > > Anyway: Since both files can be decoded with unpatched FFmpeg, I > assume the demuxing hunk of your patch is unneeded, correct? > > Any reason why you cannot use mov for muxing? > > Carl Eugen > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Disable MSA optimization for big endian arch
Shivraj: yes, -mmsa flag will be added and should not be an issue for big endian mips builds. > +if enabled bigendian && enabled msa; then > +disable msa > +fi As currently, MSA optimizations does not support big endian, above code will disable MSA and switch to default C functions. -Original Message- From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of Michael Niedermayer Sent: 16 May 2017 20:22 To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH] Disable MSA optimization for big endian arch On Mon, Apr 24, 2017 at 05:33:22PM +0530, shivraj.pa...@imgtec.com wrote: > From: Shivraj Patil > > Signed-off-by: Shivraj Patil > --- > configure |4 > 1 file changed, 4 insertions(+) > > diff --git a/configure b/configure > index 1e3463c..c63a48a 100755 > --- a/configure > +++ b/configure > @@ -5357,6 +5357,10 @@ elif enabled mips; then > enabled mipsdsp && check_inline_asm_flags mipsdsp '"addu.qb $t0, $t1, > $t2"' '-mdsp' > enabled mipsdspr2 && check_inline_asm_flags mipsdspr2 '"absq_s.qb $t0, > $t1"' '-mdspr2' > > +if enabled bigendian && enabled msa; then > +disable msa > +fi the order of this looks a bit odd for example there is above: enabled mipsfpu && enabled msa && check_inline_asm_flags msa '"addvi.b $w0, $w1, 1"' '-mmsa' && check_header msa.h || disable msa I think this would add -mmsa to the flags or disable msa already with the code you add msa is disabled but -mmsa is left in the flags Please correct me if iam wrong. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No snowflake in an avalanche ever feels responsible. -- Voltaire ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: If vfwcap_indev disabled, don't do vfw32 checks; if enabled, require vfw32
On Tue, May 16, 2017 at 03:38:32PM -0700, Aaron Levinson wrote: > Signed-off-by: Aaron Levinson > --- > configure | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/configure b/configure > index 7980d92..49f91ac 100755 > --- a/configure > +++ b/configure > @@ -6013,10 +6013,10 @@ check_header sys/videoio.h > check_code cc sys/videoio.h "struct v4l2_frmsizeenum vfse; > vfse.discrete.width = 0;" && enable_safe struct_v4l2_frmivalenum_discrete > > check_lib user32 "windows.h winuser.h" GetShellWindow -luser32 > -check_lib vfw32 "windows.h vfw.h" capCreateCaptureWindow -lvfw32 > +disabled vfwcap_indev || { require vfw32 "windows.h vfw.h" > capCreateCaptureWindow -lvfw32 && > # check that WM_CAP_DRIVER_CONNECT is defined to the proper value > # w32api 3.12 had it defined wrong > -check_cpp_condition vfw.h "WM_CAP_DRIVER_CONNECT > WM_USER" && enable > vfwcap_defines > + check_cpp_condition vfw.h "WM_CAP_DRIVER_CONNECT > > WM_USER" && enable vfwcap_defines; } > this breaks build on linux ERROR: vfw32 not found If you think configure made a mistake, make sure you are using the latest version from Git. If the latest version fails, report the problem to the ffmpeg-u...@ffmpeg.org mailing list or IRC #ffmpeg on irc.freenode.net. Include the log file "ffbuild/config.log" produced by configure as this will help solve the problem. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you drop bombs on a foreign country and kill a hundred thousand innocent people, expect your government to call the consequence "unprovoked inhuman terrorist attacks" and use it to justify dropping more bombs and killing more people. The technology changed, the idea is old. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
Should fix Ticket6349. Modifying data pointer may make it unaligned. Also change frame->nb_samples < max to frame->nb_samples <= max. This improves performance. Benchmark: ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null null old: 25767 decicycles in take_samples,1023 runs, 1 skips 25422 decicycles in take_samples,2047 runs, 1 skips 25181 decicycles in take_samples,4095 runs, 1 skips 24904 decicycles in take_samples,8191 runs, 1 skips new: 550 decicycles in take_samples,1024 runs, 0 skips 548 decicycles in take_samples,2048 runs, 0 skips 545 decicycles in take_samples,4096 runs, 0 skips 544 decicycles in take_samples,8192 runs, 0 skips Signed-off-by: Muhammad Faiz --- libavfilter/avfilter.c | 3 ++- libavfilter/framequeue.c | 2 ++ libavfilter/framequeue.h | 5 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index 08b86b0..1b6c432 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned min, unsigned max, called with enough samples. */ av_assert1(samples_ready(link, link->min_samples)); frame0 = frame = ff_framequeue_peek(&link->fifo, 0); -if (frame->nb_samples >= min && frame->nb_samples < max) { +if (!link->fifo.samples_skipped && frame->nb_samples >= min && frame->nb_samples <= max) { *rframe = ff_framequeue_take(&link->fifo); return 0; } @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, AVFrame **rframe) *rframe = NULL; if (!ff_inlink_check_available_frame(link)) return 0; +av_assert1(!link->fifo.samples_skipped); frame = ff_framequeue_take(&link->fifo); consume_update(link, frame); *rframe = frame; diff --git a/libavfilter/framequeue.c b/libavfilter/framequeue.c index 26bfa49..fed1118 100644 --- a/libavfilter/framequeue.c +++ b/libavfilter/framequeue.c @@ -107,6 +107,7 @@ AVFrame *ff_framequeue_take(FFFrameQueue *fq) fq->tail &= fq->allocated - 1; fq->total_frames_tail++; fq->total_samples_tail += b->frame->nb_samples; +fq->samples_skipped = 0; check_consistency(fq); return b->frame; } @@ -146,5 +147,6 @@ void ff_framequeue_skip_samples(FFFrameQueue *fq, size_t samples, AVRational tim for (i = 0; i < planes && i < AV_NUM_DATA_POINTERS; i++) b->frame->data[i] = b->frame->extended_data[i]; fq->total_samples_tail += samples; +fq->samples_skipped = 1; ff_framequeue_update_peeked(fq, 0); } diff --git a/libavfilter/framequeue.h b/libavfilter/framequeue.h index 5aa2c72..c49d872 100644 --- a/libavfilter/framequeue.h +++ b/libavfilter/framequeue.h @@ -100,6 +100,11 @@ typedef struct FFFrameQueue { */ uint64_t total_samples_tail; +/** + * Indicate that samples are skipped + */ +int samples_skipped; + } FFFrameQueue; /** -- 2.9.3 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: align data frame when needed
On Wed, May 17, 2017 at 4:06 PM, Muhammad Faiz wrote: > On Wed, May 17, 2017 at 3:20 PM, Nicolas George wrote: >> L'octidi 28 floréal, an CCXXV, Muhammad Faiz a écrit : >>> Of course, pushing the partial fix won't conflict with the complete >>> fix. >> >> Of course it will, it touches the same area of code. > > No, your code doesn't check the alignment at take_samples. > >> >>> Ha ha ha. Of course, that's why I prefer the partial fix. Because it >>> is simple. >> >> It is simple because it is incomplete. > > But it is enough to fix ticket 6349. > >> >>> We assume that at first data and linesize are always >>> aligned, so we only need to check data[0] alignment. >> >> Untrue. You only need to check data[0] because you only want to fix a >> tiny case of the more general bug. > > With the assumption that ff_framequeue_skip_samples is the only source > of unaligned audio data and the framework doesn't mix between > ff_inlink_consume_frame and ff_inlink_consume_samples, this partial > fix is enough to fix ticket 6349. > > Thank's. A better patch has been posted. Thank's. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : > Should fix Ticket6349. > Modifying data pointer may make it unaligned. > > Also change frame->nb_samples < max to frame->nb_samples <= max. > This improves performance. Benchmark: > ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null > null > old: > 25767 decicycles in take_samples,1023 runs, 1 skips > 25422 decicycles in take_samples,2047 runs, 1 skips > 25181 decicycles in take_samples,4095 runs, 1 skips > 24904 decicycles in take_samples,8191 runs, 1 skips > > new: > 550 decicycles in take_samples,1024 runs, 0 skips > 548 decicycles in take_samples,2048 runs, 0 skips > 545 decicycles in take_samples,4096 runs, 0 skips > 544 decicycles in take_samples,8192 runs, 0 skips > > Signed-off-by: Muhammad Faiz > --- > libavfilter/avfilter.c | 3 ++- > libavfilter/framequeue.c | 2 ++ > libavfilter/framequeue.h | 5 + > 3 files changed, 9 insertions(+), 1 deletion(-) This is an interesting idea, but... > > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c > index 08b86b0..1b6c432 100644 > --- a/libavfilter/avfilter.c > +++ b/libavfilter/avfilter.c > @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned > min, unsigned max, > called with enough samples. */ > av_assert1(samples_ready(link, link->min_samples)); > frame0 = frame = ff_framequeue_peek(&link->fifo, 0); > -if (frame->nb_samples >= min && frame->nb_samples < max) { > +if (!link->fifo.samples_skipped && frame->nb_samples >= min && > frame->nb_samples <= max) { > *rframe = ff_framequeue_take(&link->fifo); > return 0; > } > @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, AVFrame > **rframe) > *rframe = NULL; > if (!ff_inlink_check_available_frame(link)) > return 0; > +av_assert1(!link->fifo.samples_skipped); ... I am pretty sure that this assert can fail. Not with the current code, but with future filters that use the ff_inlink API directly. > frame = ff_framequeue_take(&link->fifo); > consume_update(link, frame); > *rframe = frame; > diff --git a/libavfilter/framequeue.c b/libavfilter/framequeue.c > index 26bfa49..fed1118 100644 > --- a/libavfilter/framequeue.c > +++ b/libavfilter/framequeue.c > @@ -107,6 +107,7 @@ AVFrame *ff_framequeue_take(FFFrameQueue *fq) > fq->tail &= fq->allocated - 1; > fq->total_frames_tail++; > fq->total_samples_tail += b->frame->nb_samples; > +fq->samples_skipped = 0; > check_consistency(fq); > return b->frame; > } > @@ -146,5 +147,6 @@ void ff_framequeue_skip_samples(FFFrameQueue *fq, size_t > samples, AVRational tim > for (i = 0; i < planes && i < AV_NUM_DATA_POINTERS; i++) > b->frame->data[i] = b->frame->extended_data[i]; > fq->total_samples_tail += samples; > +fq->samples_skipped = 1; > ff_framequeue_update_peeked(fq, 0); > } > diff --git a/libavfilter/framequeue.h b/libavfilter/framequeue.h > index 5aa2c72..c49d872 100644 > --- a/libavfilter/framequeue.h > +++ b/libavfilter/framequeue.h > @@ -100,6 +100,11 @@ typedef struct FFFrameQueue { > */ > uint64_t total_samples_tail; > > +/** > + * Indicate that samples are skipped > + */ > +int samples_skipped; > + > } FFFrameQueue; > > /** Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
On 5/18/17, Nicolas George wrote: > Le nonidi 29 floreal, an CCXXV, Muhammad Faiz a ecrit : >> Should fix Ticket6349. >> Modifying data pointer may make it unaligned. >> >> Also change frame->nb_samples < max to frame->nb_samples <= max. >> This improves performance. Benchmark: >> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null >> null >> old: >> 25767 decicycles in take_samples,1023 runs, 1 skips >> 25422 decicycles in take_samples,2047 runs, 1 skips >> 25181 decicycles in take_samples,4095 runs, 1 skips >> 24904 decicycles in take_samples,8191 runs, 1 skips >> >> new: >> 550 decicycles in take_samples,1024 runs, 0 skips >> 548 decicycles in take_samples,2048 runs, 0 skips >> 545 decicycles in take_samples,4096 runs, 0 skips >> 544 decicycles in take_samples,8192 runs, 0 skips >> >> Signed-off-by: Muhammad Faiz >> --- >> libavfilter/avfilter.c | 3 ++- >> libavfilter/framequeue.c | 2 ++ >> libavfilter/framequeue.h | 5 + >> 3 files changed, 9 insertions(+), 1 deletion(-) > > This is an interesting idea, but... > >> >> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c >> index 08b86b0..1b6c432 100644 >> --- a/libavfilter/avfilter.c >> +++ b/libavfilter/avfilter.c >> @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned >> min, unsigned max, >> called with enough samples. */ >> av_assert1(samples_ready(link, link->min_samples)); >> frame0 = frame = ff_framequeue_peek(&link->fifo, 0); >> -if (frame->nb_samples >= min && frame->nb_samples < max) { >> +if (!link->fifo.samples_skipped && frame->nb_samples >= min && >> frame->nb_samples <= max) { >> *rframe = ff_framequeue_take(&link->fifo); >> return 0; >> } >> @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, >> AVFrame **rframe) >> *rframe = NULL; >> if (!ff_inlink_check_available_frame(link)) >> return 0; > >> +av_assert1(!link->fifo.samples_skipped); > > ... I am pretty sure that this assert can fail. Not with the current > code, but with future filters that use the ff_inlink API directly. Missingle single thing about future filters, and why would they use ff_inlink API directly. If you can not cooperate, have very short time to work on FFmpeg, can not stand criticism of other FFmpeg developers,.. just leave the project for once. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter: document incompatibility between ff_inlink_consume_frame/samples
nearly copy paste from buffersink.h Signed-off-by: Muhammad Faiz --- libavfilter/filters.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavfilter/filters.h b/libavfilter/filters.h index 2c78d60..c695fbf 100644 --- a/libavfilter/filters.h +++ b/libavfilter/filters.h @@ -92,6 +92,9 @@ int ff_inlink_consume_frame(AVFilterLink *link, AVFrame **rframe); * @return >0 if a frame is available, * 0 and set rframe to NULL if no frame available, * or AVERROR code + * + * @warning do not mix this function with ff_inlink_consume_frame(). Use only one or + * the other with a single link, not both. */ int ff_inlink_consume_samples(AVFilterLink *link, unsigned min, unsigned max, AVFrame **rframe); -- 2.9.3 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
On 5/18/2017 12:49 PM, Paul B Mahol wrote: > On 5/18/17, Nicolas George wrote: >> Le nonidi 29 floreal, an CCXXV, Muhammad Faiz a ecrit : >>> Should fix Ticket6349. >>> Modifying data pointer may make it unaligned. >>> >>> Also change frame->nb_samples < max to frame->nb_samples <= max. >>> This improves performance. Benchmark: >>> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null >>> null >>> old: >>> 25767 decicycles in take_samples,1023 runs, 1 skips >>> 25422 decicycles in take_samples,2047 runs, 1 skips >>> 25181 decicycles in take_samples,4095 runs, 1 skips >>> 24904 decicycles in take_samples,8191 runs, 1 skips >>> >>> new: >>> 550 decicycles in take_samples,1024 runs, 0 skips >>> 548 decicycles in take_samples,2048 runs, 0 skips >>> 545 decicycles in take_samples,4096 runs, 0 skips >>> 544 decicycles in take_samples,8192 runs, 0 skips >>> >>> Signed-off-by: Muhammad Faiz >>> --- >>> libavfilter/avfilter.c | 3 ++- >>> libavfilter/framequeue.c | 2 ++ >>> libavfilter/framequeue.h | 5 + >>> 3 files changed, 9 insertions(+), 1 deletion(-) >> >> This is an interesting idea, but... >> >>> >>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c >>> index 08b86b0..1b6c432 100644 >>> --- a/libavfilter/avfilter.c >>> +++ b/libavfilter/avfilter.c >>> @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned >>> min, unsigned max, >>> called with enough samples. */ >>> av_assert1(samples_ready(link, link->min_samples)); >>> frame0 = frame = ff_framequeue_peek(&link->fifo, 0); >>> -if (frame->nb_samples >= min && frame->nb_samples < max) { >>> +if (!link->fifo.samples_skipped && frame->nb_samples >= min && >>> frame->nb_samples <= max) { >>> *rframe = ff_framequeue_take(&link->fifo); >>> return 0; >>> } >>> @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, >>> AVFrame **rframe) >>> *rframe = NULL; >>> if (!ff_inlink_check_available_frame(link)) >>> return 0; >> >>> +av_assert1(!link->fifo.samples_skipped); >> >> ... I am pretty sure that this assert can fail. Not with the current >> code, but with future filters that use the ff_inlink API directly. > > Missingle single thing about future filters, and why would they use > ff_inlink API > directly. > > If you can not cooperate, have very short time to work on FFmpeg, can not > stand > criticism of other FFmpeg developers,.. just leave the project for once. Let's work on a solution instead of fighting and shit flinging for once... ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
On Thu, May 18, 2017 at 10:56 PM, James Almer wrote: > On 5/18/2017 12:49 PM, Paul B Mahol wrote: >> On 5/18/17, Nicolas George wrote: >>> Le nonidi 29 floreal, an CCXXV, Muhammad Faiz a ecrit : Should fix Ticket6349. Modifying data pointer may make it unaligned. Also change frame->nb_samples < max to frame->nb_samples <= max. This improves performance. Benchmark: ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null null old: 25767 decicycles in take_samples,1023 runs, 1 skips 25422 decicycles in take_samples,2047 runs, 1 skips 25181 decicycles in take_samples,4095 runs, 1 skips 24904 decicycles in take_samples,8191 runs, 1 skips new: 550 decicycles in take_samples,1024 runs, 0 skips 548 decicycles in take_samples,2048 runs, 0 skips 545 decicycles in take_samples,4096 runs, 0 skips 544 decicycles in take_samples,8192 runs, 0 skips Signed-off-by: Muhammad Faiz --- libavfilter/avfilter.c | 3 ++- libavfilter/framequeue.c | 2 ++ libavfilter/framequeue.h | 5 + 3 files changed, 9 insertions(+), 1 deletion(-) >>> >>> This is an interesting idea, but... >>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index 08b86b0..1b6c432 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned min, unsigned max, called with enough samples. */ av_assert1(samples_ready(link, link->min_samples)); frame0 = frame = ff_framequeue_peek(&link->fifo, 0); -if (frame->nb_samples >= min && frame->nb_samples < max) { +if (!link->fifo.samples_skipped && frame->nb_samples >= min && frame->nb_samples <= max) { *rframe = ff_framequeue_take(&link->fifo); return 0; } @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, AVFrame **rframe) *rframe = NULL; if (!ff_inlink_check_available_frame(link)) return 0; >>> +av_assert1(!link->fifo.samples_skipped); >>> >>> ... I am pretty sure that this assert can fail. Not with the current >>> code, but with future filters that use the ff_inlink API directly. >> >> Missingle single thing about future filters, and why would they use >> ff_inlink API >> directly. >> >> If you can not cooperate, have very short time to work on FFmpeg, can not >> stand >> criticism of other FFmpeg developers,.. just leave the project for once. > > Let's work on a solution instead of fighting and shit flinging for once... I'm sorry. I've answered to Nicolas (falsely to Nicolas, not to ffmpeg-devel, that's my fault), and he gives positive review. Thank's. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
On Thu, May 18, 2017 at 10:32 PM, Muhammad Faiz wrote: > On Thu, May 18, 2017 at 9:59 PM, Nicolas George wrote: >> Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >>> Should fix Ticket6349. >>> Modifying data pointer may make it unaligned. >>> >>> Also change frame->nb_samples < max to frame->nb_samples <= max. >>> This improves performance. Benchmark: >>> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null >>> null >>> old: >>> 25767 decicycles in take_samples,1023 runs, 1 skips >>> 25422 decicycles in take_samples,2047 runs, 1 skips >>> 25181 decicycles in take_samples,4095 runs, 1 skips >>> 24904 decicycles in take_samples,8191 runs, 1 skips >>> >>> new: >>> 550 decicycles in take_samples,1024 runs, 0 skips >>> 548 decicycles in take_samples,2048 runs, 0 skips >>> 545 decicycles in take_samples,4096 runs, 0 skips >>> 544 decicycles in take_samples,8192 runs, 0 skips >>> >>> Signed-off-by: Muhammad Faiz >>> --- >>> libavfilter/avfilter.c | 3 ++- >>> libavfilter/framequeue.c | 2 ++ >>> libavfilter/framequeue.h | 5 + >>> 3 files changed, 9 insertions(+), 1 deletion(-) >> >> This is an interesting idea, but... >> >>> >>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c >>> index 08b86b0..1b6c432 100644 >>> --- a/libavfilter/avfilter.c >>> +++ b/libavfilter/avfilter.c >>> @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned >>> min, unsigned max, >>> called with enough samples. */ >>> av_assert1(samples_ready(link, link->min_samples)); >>> frame0 = frame = ff_framequeue_peek(&link->fifo, 0); >>> -if (frame->nb_samples >= min && frame->nb_samples < max) { >>> +if (!link->fifo.samples_skipped && frame->nb_samples >= min && >>> frame->nb_samples <= max) { >>> *rframe = ff_framequeue_take(&link->fifo); >>> return 0; >>> } >>> @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, >>> AVFrame **rframe) >>> *rframe = NULL; >>> if (!ff_inlink_check_available_frame(link)) >>> return 0; >> >>> +av_assert1(!link->fifo.samples_skipped); >> >> ... I am pretty sure that this assert can fail. Not with the current >> code, but with future filters that use the ff_inlink API directly. > > IMHO, the solution is to document it properly to not mix > ff_inlink_consume_samples with ff_inlink_consume_frame, similar to > av_buffersink_get_frame vs av_buffersink_get_samples. > > Thank's. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
On Thu, May 18, 2017 at 10:37 PM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >> >> Should fix Ticket6349. > > Forgot: please remove that, it is not true. The bug in this ticket is in > libavcodec, it cannot be fixed by a change in libavfilter. In my opinion, it can. The bigger problem about alignment should be separated from this ticket e.g opening new ticket that gives failed alignment test case. > >> IMHO, the solution is to document it properly to not mix >> ff_inlink_consume_samples with ff_inlink_consume_frame, similar to >> av_buffersink_get_frame vs av_buffersink_get_samples. > > Maybe. I am not sure I like this solution, but it should be technically > acceptable, unlike no change at all. I would like better something that > gives the performance boost you observe without limiting the > expressiveness of the API, but I do not know if it is possible. A patch documenting it has been posted. Thank's. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/graphparser: allow specifying filter@id as filter instance
On 5/17/17, Muhammad Faiz wrote: > See http://lists.ffmpeg.org/pipermail/ffmpeg-user/2017-April/035975.html > Parsed_filter_X is not intuitive as filter instance name and > also undocumented. > > Signed-off-by: Muhammad Faiz > --- > doc/filters.texi | 13 ++--- > libavfilter/graphparser.c | 26 +- > 2 files changed, 31 insertions(+), 8 deletions(-) > Could you add example in git log when you also linked that thread article. IMHO Parsed could remain and user can override it with custom one. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: If vfwcap_indev disabled, don't do vfw32 checks; if enabled, require vfw32
On Thu, May 18, 2017 at 3:18 PM, Michael Niedermayer wrote: > On Tue, May 16, 2017 at 03:38:32PM -0700, Aaron Levinson wrote: >> Signed-off-by: Aaron Levinson >> --- >> configure | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/configure b/configure >> index 7980d92..49f91ac 100755 >> --- a/configure >> +++ b/configure >> @@ -6013,10 +6013,10 @@ check_header sys/videoio.h >> check_code cc sys/videoio.h "struct v4l2_frmsizeenum vfse; >> vfse.discrete.width = 0;" && enable_safe struct_v4l2_frmivalenum_discrete >> >> check_lib user32 "windows.h winuser.h" GetShellWindow -luser32 >> -check_lib vfw32 "windows.h vfw.h" capCreateCaptureWindow -lvfw32 >> +disabled vfwcap_indev || { require vfw32 "windows.h vfw.h" >> capCreateCaptureWindow -lvfw32 && >> # check that WM_CAP_DRIVER_CONNECT is defined to the proper value >> # w32api 3.12 had it defined wrong >> -check_cpp_condition vfw.h "WM_CAP_DRIVER_CONNECT > WM_USER" && enable >> vfwcap_defines >> + check_cpp_condition vfw.h "WM_CAP_DRIVER_CONNECT >> > WM_USER" && enable vfwcap_defines; } >> > > this breaks build on linux > ERROR: vfw32 not found > As discussed on IRC, other then the error on linux, overall I'm not sure what exactly this change improves? vfw detection seems to work just fine as is. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/graphparser: allow specifying filter@id as filter instance
On Thu, May 18, 2017 at 11:11 PM, Paul B Mahol wrote: > On 5/17/17, Muhammad Faiz wrote: >> See http://lists.ffmpeg.org/pipermail/ffmpeg-user/2017-April/035975.html >> Parsed_filter_X is not intuitive as filter instance name and >> also undocumented. >> >> Signed-off-by: Muhammad Faiz >> --- >> doc/filters.texi | 13 ++--- >> libavfilter/graphparser.c | 26 +- >> 2 files changed, 31 insertions(+), 8 deletions(-) >> > > Could you add example in git log when you also linked that thread > article. Added Example: ffplay -f lavfi "nullsrc=s=640x360, sendcmd='1 drawtext@top reinit text=Hello; 2 drawtext@bottom reinit text=World', drawtext@top=x=16:y=16:fontsize=20:fontcolor=Red:text='', drawtext@bottom=x=16:y=340:fontsize=16:fontcolor=Blue:text=''" > > IMHO Parsed could remain and user can override it with custom one. Changed. Thank's ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/graphparser: allow specifying filter@id as filter instance
On 5/18/17, Muhammad Faiz wrote: > On Thu, May 18, 2017 at 11:11 PM, Paul B Mahol wrote: >> On 5/17/17, Muhammad Faiz wrote: >>> See http://lists.ffmpeg.org/pipermail/ffmpeg-user/2017-April/035975.html >>> Parsed_filter_X is not intuitive as filter instance name and >>> also undocumented. >>> >>> Signed-off-by: Muhammad Faiz >>> --- >>> doc/filters.texi | 13 ++--- >>> libavfilter/graphparser.c | 26 +- >>> 2 files changed, 31 insertions(+), 8 deletions(-) >>> >> >> Could you add example in git log when you also linked that thread >> article. > > Added > Example: > ffplay -f lavfi "nullsrc=s=640x360, > sendcmd='1 drawtext@top reinit text=Hello; 2 drawtext@bottom reinit > text=World', > drawtext@top=x=16:y=16:fontsize=20:fontcolor=Red:text='', > drawtext@bottom=x=16:y=340:fontsize=16:fontcolor=Blue:text=''" > >> >> IMHO Parsed could remain and user can override it with custom one. > > Changed. > > Thank's No more comments from me. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : > In my opinion, it can. No, it cannot: the frame that crashes goes from the application to libavcodec. It came earlier from libavfilter, but that is irrelevant: an application could have done the same thing without libavfilter, and resulted in the same crash while completely compatible with the documented API. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
Le nonidi 29 floréal, an CCXXV, James Almer a écrit : > Let's work on a solution instead of fighting and shit flinging for once... Thank you. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH/WIP] avcodec/x86: move simple_idct to external assembly
Hi, cursory review, I appreciate that this is work in progress so don't read too much into it. General points: - I really like how you're doing a literal translation without trying to change it, this makes review easier and also reduces the likelihood that there's performance implications. - do you think a checkasm test makes sense? That would also make performance measuring easier. Code review: On Fri, May 12, 2017 at 10:27 AM, James Darnley wrote: > +; Simple IDCT MMX > +; > +; Copyright (c) 2001, 2002 Michael Niedermayer > Please add a line that you converted it from inline asm to x264asm syntax, we've done that in other places also. > +%macro DC_COND_IDCT 7 > +movq mm0, [blockq + %1] ; R4 R0 r4 r0 > +movq mm1, [blockq + %2] ; R6 R2 r6 r2 > +movq mm2, [blockq + %3] ; R3 R1 r3 r1 > +movq mm3, [blockq + %4] ; R7 R5 r7 r5 > Please use 4-space indentation. > +%%9: > +; :: "r" (block), "r" (temp), "r" (coeffs) > +;NAMED_CONSTRAINTS_ADD(wm1010,d4) > +; : "%eax" > +%endmacro > The inline asm bits (middle 3 lines) can be removed (yay!). Rest is fine. I am assuming that the binary size will grow slightly because the idct is now inlined in the put/add (and duplicated between mmx/sse2), but no performance implication (or possibly slight improvement because of the inline). If that's correct, I'm OK with this. Are you planning to write an actual SSE2 version of the IDCT? (No pressure, just wondering.) Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: If vfwcap_indev disabled, don't do vfw32 checks; if enabled, require vfw32
On 5/18/2017 9:21 AM, Hendrik Leppkes wrote: On Thu, May 18, 2017 at 3:18 PM, Michael Niedermayer wrote: On Tue, May 16, 2017 at 03:38:32PM -0700, Aaron Levinson wrote: Signed-off-by: Aaron Levinson --- configure | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 7980d92..49f91ac 100755 --- a/configure +++ b/configure @@ -6013,10 +6013,10 @@ check_header sys/videoio.h check_code cc sys/videoio.h "struct v4l2_frmsizeenum vfse; vfse.discrete.width = 0;" && enable_safe struct_v4l2_frmivalenum_discrete check_lib user32 "windows.h winuser.h" GetShellWindow -luser32 -check_lib vfw32 "windows.h vfw.h" capCreateCaptureWindow -lvfw32 +disabled vfwcap_indev || { require vfw32 "windows.h vfw.h" capCreateCaptureWindow -lvfw32 && # check that WM_CAP_DRIVER_CONNECT is defined to the proper value # w32api 3.12 had it defined wrong -check_cpp_condition vfw.h "WM_CAP_DRIVER_CONNECT > WM_USER" && enable vfwcap_defines + check_cpp_condition vfw.h "WM_CAP_DRIVER_CONNECT > WM_USER" && enable vfwcap_defines; } this breaks build on linux ERROR: vfw32 not found As discussed on IRC, other then the error on linux, overall I'm not sure what exactly this change improves? vfw detection seems to work just fine as is. Agree--thanks for looking at this. Please disregard this patch--it is incorrect in a couple of different ways. Aaron Levinson ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] New to the list... thinking about a feature
On Thu, 18 May 2017 12:20:55 +0200, Pedro Andres Aranda Gutierrez wrote: > I'm completely new to the list but have a couple of years experience as a > user. I happen to work a lot with things I record from satellite. My > recorder will include a lot of empty streams in the transport stream and > that is annoying. I have tweaked ffmpeg here and there to show only > non-empty streams at the 'normal' verbosity level and include empty streams > at heightened verbosity. depending on the complexity of the patch. and if it does not break other files. and assuming it passes fate... just submit patch here , we'll review it. even if ffmpeg does not accept it, its possible that other people will use your patch for their setup. thanks! welcome to ffmpeg. -compn ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avio_read data loss with multiple calls to fill_buffer
avio_read may make multiple calls to fill_buffer to accumulate bytes to fulfill a request. fill_buffer will overwrite previously read data if it is not prepped. Added a call to 'ffio_ensure_seekback' to adjust the s->buffer to fill_buffer's liking. This isn't necessary for the very first call to fill_buffer, thus the "size1 != size" check. --- libavformat/aviobuf.c | 5 + 1 file changed, 5 insertions(+) diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index 0a7c39eacd..8e9594cab8 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -648,6 +648,11 @@ int avio_read(AVIOContext *s, unsigned char *buf, int size) s->buf_end = s->buffer/* + len*/; } } else { +if (size1 != size) +/* this call will ensure fill_buffer will not overwrite previously +read data. */ +ffio_ensure_seekback(s, size1); + fill_buffer(s); len = s->buf_end - s->buf_ptr; if (len == 0) -- 2.13.0.303.g4ebf302169-goog ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avio_read data loss with multiple calls to fill_buffer
On Thu, May 18, 2017 at 01:12:49PM -0700, Rob Meyers wrote: > avio_read may make multiple calls to fill_buffer to accumulate bytes > to fulfill a request. fill_buffer will overwrite previously read data > if it is not prepped. Added a call to 'ffio_ensure_seekback' to adjust > the s->buffer to fill_buffer's liking. This isn't necessary for the > very first call to fill_buffer, thus the "size1 != size" check. > --- > libavformat/aviobuf.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c > index 0a7c39eacd..8e9594cab8 100644 > --- a/libavformat/aviobuf.c > +++ b/libavformat/aviobuf.c > @@ -648,6 +648,11 @@ int avio_read(AVIOContext *s, unsigned char *buf, int > size) > s->buf_end = s->buffer/* + len*/; > } > } else { > +if (size1 != size) > +/* this call will ensure fill_buffer will not overwrite > previously > +read data. */ > +ffio_ensure_seekback(s, size1); ffio_ensure_seekback() should be called when theres a potential seek back in the following code. There is no seekback in avio_read(), avio_read only moves forward. Some callers may do a avio_read() and or other function calls and then seek back. A caller might do 3 avio_read() and seek back over all ffio_ensure_seekback() generally should be called by the code that later initiates teh seek back ffio_ensure_seekback() is not free it should only be used when it is needed. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avio_read data loss with multiple calls to fill_buffer
avio_read does multiple calls to fill_buffer(). Adjusting things in fill_buffer was frowned upon; how would you suggest I approach this? The id3v2 seek doesn't come into play; the return from avio_read is the "full" read. The call I added in avio_read is only done when there was accumulated data, so avoid reallocating the buffer the first time through. On Thu, May 18, 2017 at 3:53 PM Michael Niedermayer wrote: > On Thu, May 18, 2017 at 01:12:49PM -0700, Rob Meyers wrote: > > avio_read may make multiple calls to fill_buffer to accumulate bytes > > to fulfill a request. fill_buffer will overwrite previously read data > > if it is not prepped. Added a call to 'ffio_ensure_seekback' to adjust > > the s->buffer to fill_buffer's liking. This isn't necessary for the > > very first call to fill_buffer, thus the "size1 != size" check. > > --- > > libavformat/aviobuf.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c > > index 0a7c39eacd..8e9594cab8 100644 > > --- a/libavformat/aviobuf.c > > +++ b/libavformat/aviobuf.c > > @@ -648,6 +648,11 @@ int avio_read(AVIOContext *s, unsigned char *buf, > int size) > > s->buf_end = s->buffer/* + len*/; > > } > > } else { > > +if (size1 != size) > > +/* this call will ensure fill_buffer will not > overwrite previously > > +read data. */ > > +ffio_ensure_seekback(s, size1); > > ffio_ensure_seekback() should be called when theres a potential > seek back in the following code. > > There is no seekback in avio_read(), avio_read only moves forward. > Some callers may do a avio_read() and or other function calls and > then seek back. A caller might do 3 avio_read() and seek back over all > > ffio_ensure_seekback() generally should be called by the code that > later initiates teh seek back > > ffio_ensure_seekback() is not free it should only be used when it is > needed. > > > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Those who are too smart to engage in politics are punished by being > governed by those who are dumber. -- Plato > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped
On Thu, May 18, 2017 at 11:59 PM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >> In my opinion, it can. > > No, it cannot: the frame that crashes goes from the application to > libavcodec. It came earlier from libavfilter, but that is irrelevant: an > application could have done the same thing without libavfilter, and > resulted in the same crash while completely compatible with the > documented API. Every patch that can fix the crash of ffmpeg -i clip.wav -af alimiter -c:a mp3 -b:a 128k -ar 48k -f null - can claim that it fixes ticket 6349. Other cases should be in separate tickets. Thank's. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/6] avformat/movenc: always check for new extradata on a packet
On 4/13/2017 4:54 PM, James Almer wrote: > Don't just look at zero sized packets, and also check for AAC extradata > updates, in preparation for the following patches. > > Signed-off-by: James Almer > --- > libavformat/movenc.c | 34 ++ > 1 file changed, 18 insertions(+), 16 deletions(-) Ping for the set. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH V3] lavc/vaapi_encode_h264: Enable MB rate control.
On 2017/5/14 12:26, Jun Zhao wrote: > V3: - Fix build error with old VAAPI version. > V2: - Refine the name/value type to mb_rate_control/bool. > - Only supported GEN9+ (SKL/APL/KBL/...) > - i965 driver default use frame-level rate control algorithm (generate > the QP for each frame), > when enable mb_rate_control, it's will enable the MB-level RC algorithm > (generate the QP for each MB). > - enables MB-level bitrate control that generally improves subjective > visual quality, > but have negative impact on performance and objective visual quality > metric. > Ping ? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] New to the list... thinking about a feature
2017-05-18 12:20 GMT+02:00 Pedro Andres Aranda Gutierrez : > I'm completely new to the list but have a couple of years experience as a > user. I happen to work a lot with things I record from satellite. My > recorder will include a lot of empty streams in the transport stream and > that is annoying. I have tweaked ffmpeg here and there to show only > non-empty streams at the 'normal' verbosity level and include empty streams > at heightened verbosity. > > Is that of interest? Even if your change is not acceptable (we cannot know so far but I believe your change was requested repeatedly), this mailing list is the right place to archive it. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel