Re: [FFmpeg-devel] [PATCH] avcodec/mpeg4videodec: Use more specific error codes
On 3/10/2018 6:15 PM, Michael Niedermayer wrote: > On Sat, Mar 10, 2018 at 03:33:33PM -0300, James Almer wrote: >> On 3/10/2018 2:34 PM, Michael Niedermayer wrote: >>> Signed-off-by: Michael Niedermayer >>> --- >>> libavcodec/mpeg4videodec.c | 100 >>> +++-- >>> 1 file changed, 51 insertions(+), 49 deletions(-) >>> >>> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c >>> index 19210d97fe..1357b357a8 100644 >>> --- a/libavcodec/mpeg4videodec.c >>> +++ b/libavcodec/mpeg4videodec.c >>> @@ -448,7 +448,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext >>> *ctx) >>> >>> /* is there enough space left for a video packet + header */ >>> if (get_bits_count(&s->gb) > s->gb.size_in_bits - 20) >>> -return -1; >>> +return AVERROR_INVALIDDATA; >>> >>> for (len = 0; len < 32; len++) >>> if (get_bits1(&s->gb)) >>> @@ -456,7 +456,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext >>> *ctx) >>> >>> if (len != ff_mpeg4_get_video_packet_prefix_length(s)) { >>> av_log(s->avctx, AV_LOG_ERROR, "marker does not match f_code\n"); >>> -return -1; >>> +return AVERROR_INVALIDDATA; >>> } >>> >>> if (ctx->shape != RECT_SHAPE) { >>> @@ -468,7 +468,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext >>> *ctx) >>> if (mb_num >= s->mb_num || !mb_num) { >>> av_log(s->avctx, AV_LOG_ERROR, >>> "illegal mb_num in video packet (%d %d) \n", mb_num, >>> s->mb_num); >>> -return -1; >>> +return AVERROR_INVALIDDATA; >>> } >>> >>> s->mb_x = mb_num % s->mb_width; >>> @@ -597,7 +597,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, >>> int n, int *dir_ptr) >>> >>> if (code < 0 || code > 9 /* && s->nbit < 9 */) { >>> av_log(s->avctx, AV_LOG_ERROR, "illegal dc vlc\n"); >>> -return -1; >>> +return AVERROR_INVALIDDATA; >>> } >>> >>> if (code == 0) { >>> @@ -620,7 +620,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, >>> int n, int *dir_ptr) >>> if (get_bits1(&s->gb) == 0) { /* marker */ >>> if (s->avctx->err_recognition & >>> (AV_EF_BITSTREAM|AV_EF_COMPLIANT)) { >>> av_log(s->avctx, AV_LOG_ERROR, "dc marker bit >>> missing\n"); >>> -return -1; >>> +return AVERROR_INVALIDDATA; >>> } >>> } >>> } >>> @@ -664,7 +664,7 @@ static int mpeg4_decode_partition_a(Mpeg4DecContext >>> *ctx) >>> if (cbpc < 0) { >>> av_log(s->avctx, AV_LOG_ERROR, >>> "mcbpc corrupted at %d %d\n", s->mb_x, >>> s->mb_y); >>> -return -1; >>> +return cbpc; >> >> get_vlc2() seems to return -1 on error, so nothing really changes with >> this. > > right, ill hardcode these > > >> Same with every other similar call, > > the other calls should return proper error codes and we should > forward these. > Not forwarding an error because the current implementation of a > function generates an error code that the caller can hardcode is > bad design. > We should not duplicate the implementation of what a function returns > in the caller. The implementation could change and then this is wrong. > > Or am i missing a reason why hardcoding the error codes in the caller > would be an advantage ? I just meant to return AVERROR_INVALIDDATA on every other check for get_vlc2() return value, much like the one i pointed above, since those would also try to return -1 otherwise. > > will send a new patch > > thx > > [...] > > > > ___ > 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] avformat/mov: Fix integer overflows related to sample_duration
Fixes: runtime error: signed integer overflow: -9166684017437101870 + -2495066639299164439 cannot be represented in type Fixes: Chromium bug 791349 Reported-by: Matt Wolenetz Reviewed-by: Matt Wolenetz Signed-off-by: Michael Niedermayer --- libavformat/mov.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 51228f5df2..b7f9c0cdd1 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -2885,14 +2885,19 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) && total_sample_count > 100 && sample_duration/10 > duration / total_sample_count) sample_duration = duration / total_sample_count; -duration+=(int64_t)sample_duration*sample_count; +duration+=(int64_t)sample_duration*(uint64_t)sample_count; total_sample_count+=sample_count; } sc->stts_count = i; -sc->duration_for_fps += duration; -sc->nb_frames_for_fps += total_sample_count; +if (duration > 0 && +duration <= INT64_MAX - sc->duration_for_fps && +total_sample_count <= INT64_MAX - sc->nb_frames_for_fps +) { +sc->duration_for_fps += duration; +sc->nb_frames_for_fps += total_sample_count; +} if (pb->eof_reached) { av_log(c->fc, AV_LOG_WARNING, "reached eof, corrupted STTS atom\n"); @@ -4798,8 +4803,13 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) dts += sample_duration; offset += sample_size; sc->data_size += sample_size; -sc->duration_for_fps += sample_duration; -sc->nb_frames_for_fps ++; + +if (sample_duration <= INT64_MAX - sc->duration_for_fps && +1 <= INT64_MAX - sc->nb_frames_for_fps +) { +sc->duration_for_fps += sample_duration; +sc->nb_frames_for_fps ++; +} } if (i < entries) { // EOF found before reading all entries. Fix the hole this would -- 2.16.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/extract_extradata: don't uninitialize the H2645Packet on every processed packet
Based on hevc_parser code. This prevents repeated unnecessary allocations and frees on every packet processed by the bsf. Signed-off-by: James Almer --- libavcodec/extract_extradata_bsf.c | 33 +++-- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/libavcodec/extract_extradata_bsf.c b/libavcodec/extract_extradata_bsf.c index 4e2d601742..64017b6fb7 100644 --- a/libavcodec/extract_extradata_bsf.c +++ b/libavcodec/extract_extradata_bsf.c @@ -36,6 +36,9 @@ typedef struct ExtractExtradataContext { int (*extract)(AVBSFContext *ctx, AVPacket *pkt, uint8_t **data, int *size); +/* H264/HEVC specifc fields */ +H2645Packet h2645_pkt; + /* AVOptions */ int remove; } ExtractExtradataContext; @@ -61,7 +64,6 @@ static int extract_extradata_h2645(AVBSFContext *ctx, AVPacket *pkt, ExtractExtradataContext *s = ctx->priv_data; -H2645Packet h2645_pkt = { 0 }; int extradata_size = 0, filtered_size = 0; const int *extradata_nal_types; int nb_extradata_nal_types; @@ -75,13 +77,13 @@ static int extract_extradata_h2645(AVBSFContext *ctx, AVPacket *pkt, nb_extradata_nal_types = FF_ARRAY_ELEMS(extradata_nal_types_h264); } -ret = ff_h2645_packet_split(&h2645_pkt, pkt->data, pkt->size, +ret = ff_h2645_packet_split(&s->h2645_pkt, pkt->data, pkt->size, ctx, 0, 0, ctx->par_in->codec_id, 1); if (ret < 0) -goto fail; +return ret; -for (i = 0; i < h2645_pkt.nb_nals; i++) { -H2645NAL *nal = &h2645_pkt.nals[i]; +for (i = 0; i < s->h2645_pkt.nb_nals; i++) { +H2645NAL *nal = &s->h2645_pkt.nals[i]; if (val_in_array(extradata_nal_types, nb_extradata_nal_types, nal->type)) { extradata_size += nal->raw_size + 3; if (ctx->par_in->codec_id == AV_CODEC_ID_HEVC) { @@ -104,8 +106,7 @@ static int extract_extradata_h2645(AVBSFContext *ctx, AVPacket *pkt, if (s->remove) { filtered_buf = av_buffer_alloc(filtered_size + AV_INPUT_BUFFER_PADDING_SIZE); if (!filtered_buf) { -ret = AVERROR(ENOMEM); -goto fail; +return AVERROR(ENOMEM); } memset(filtered_buf->data + filtered_size, 0, AV_INPUT_BUFFER_PADDING_SIZE); @@ -115,16 +116,15 @@ static int extract_extradata_h2645(AVBSFContext *ctx, AVPacket *pkt, extradata = av_malloc(extradata_size + AV_INPUT_BUFFER_PADDING_SIZE); if (!extradata) { av_buffer_unref(&filtered_buf); -ret = AVERROR(ENOMEM); -goto fail; +return AVERROR(ENOMEM); } memset(extradata + extradata_size, 0, AV_INPUT_BUFFER_PADDING_SIZE); *data = extradata; *size = extradata_size; -for (i = 0; i < h2645_pkt.nb_nals; i++) { -H2645NAL *nal = &h2645_pkt.nals[i]; +for (i = 0; i < s->h2645_pkt.nb_nals; i++) { +H2645NAL *nal = &s->h2645_pkt.nals[i]; if (val_in_array(extradata_nal_types, nb_extradata_nal_types, nal->type)) { AV_WB24(extradata, 1); // startcode @@ -145,9 +145,7 @@ static int extract_extradata_h2645(AVBSFContext *ctx, AVPacket *pkt, } } -fail: -ff_h2645_packet_uninit(&h2645_pkt); -return ret; +return 0; } static int extract_extradata_vc1(AVBSFContext *ctx, AVPacket *pkt, @@ -311,6 +309,12 @@ fail: return ret; } +static void extract_extradata_close(AVBSFContext *ctx) +{ +ExtractExtradataContext *s = ctx->priv_data; +ff_h2645_packet_uninit(&s->h2645_pkt); +} + static const enum AVCodecID codec_ids[] = { AV_CODEC_ID_CAVS, AV_CODEC_ID_H264, @@ -343,4 +347,5 @@ const AVBitStreamFilter ff_extract_extradata_bsf = { .priv_class = &extract_extradata_class, .init = extract_extradata_init, .filter = extract_extradata_filter, +.close = extract_extradata_close, }; -- 2.16.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/extract_extradata: don't allocate more space than needed when removing NALUs in h264/hevc
On 3/10/2018 12:48 PM, James Almer wrote: > On 3/9/2018 2:40 PM, James Almer wrote: >> Signed-off-by: James Almer >> --- >> libavcodec/extract_extradata_bsf.c | 12 +++- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> > Will push this later today. > Pushed. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/wmalosslessdec: Reset num_saved_bits on error path
Fixes: NULL pointer dereference Fixes: poc-201803.wav Found-by: GwanYeong Kim Signed-off-by: Michael Niedermayer --- libavcodec/wmalosslessdec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavcodec/wmalosslessdec.c b/libavcodec/wmalosslessdec.c index 133a3e92d1..59e8929586 100644 --- a/libavcodec/wmalosslessdec.c +++ b/libavcodec/wmalosslessdec.c @@ -1148,6 +1148,7 @@ static void save_bits(WmallDecodeCtx *s, GetBitContext* gb, int len, if (len <= 0 || buflen > s->max_frame_size) { avpriv_request_sample(s->avctx, "Too small input buffer"); s->packet_loss = 1; +s->num_saved_bits = 0; return; } -- 2.16.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/rtsp: fix the type of the timeout option.
On Sat, 10 Mar 2018 20:37:20 +0100 Nicolas George wrote: > A timeout is a duration of time, therefore the correct type for it > is AV_OPT_TYPE_DURATION. It has the benefit of offering a better > user interface, with units specification. > Unfortunately, ff46124b0df17a1d35249e09ae8eae9a61f16e04 was pushed > before that mistake could be corrected. > > Signed-off-by: Nicolas George > --- > libavformat/rtsp.c | 5 +++-- > libavformat/rtsp.h | 4 > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index ceb770a3a4..1fbdcfcedd 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -98,7 +98,7 @@ const AVOption ff_rtsp_options[] = { > { "timeout", "set maximum timeout (in seconds) to wait for incoming > connections (-1 is infinite, imply flag listen) (deprecated, use > listen_timeout)", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, > INT_MIN, INT_MAX, DEC }, > { "stimeout", "set timeout (in microseconds) of socket TCP I/O > operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, > DEC }, > #else > -{ "timeout", "set timeout (in microseconds) of socket TCP I/O > operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, > DEC }, > +{ "timeout", "set timeout of socket TCP I/O operations", > OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 0}, 0, INT64_MAX, DEC }, > #endif > COMMON_OPTS(), > { "user_agent", "override User-Agent header", OFFSET(user_agent), > AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC }, > @@ -1818,7 +1818,8 @@ redirect: > /* open the tcp connection */ > ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > host, port, > -"?timeout=%d", rt->stimeout); > +/* cast necessary until FF_API_OLD_RTSP_OPTIONS removed > */ > +"?timeout=%"PRId64, (int64_t)rt->stimeout); > if ((ret = ffurl_open_whitelist(&rt->rtsp_hd, tcpname, > AVIO_FLAG_READ_WRITE, > &s->interrupt_callback, NULL, s->protocol_whitelist, > s->protocol_blacklist, NULL)) < 0) { > err = ret; > diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h > index 9a7f366b39..1524962e1b 100644 > --- a/libavformat/rtsp.h > +++ b/libavformat/rtsp.h > @@ -395,7 +395,11 @@ typedef struct RTSPState { > /** > * timeout of socket i/o operations. > */ > +#if FF_API_OLD_RTSP_OPTIONS > int stimeout; > +#else > +int64_t stimeout; > +#endif > > /** > * Size of RTP packet reordering queue. NACK ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/rtsp: fix the type of the timeout option.
On Sat, 10 Mar 2018 20:37:20 +0100 Nicolas George wrote: > A timeout is a duration of time, therefore the correct type for it > is AV_OPT_TYPE_DURATION. It has the benefit of offering a better > user interface, with units specification. > Unfortunately, ff46124b0df17a1d35249e09ae8eae9a61f16e04 was pushed > before that mistake could be corrected. > > Signed-off-by: Nicolas George > --- > libavformat/rtsp.c | 5 +++-- > libavformat/rtsp.h | 4 > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index ceb770a3a4..1fbdcfcedd 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -98,7 +98,7 @@ const AVOption ff_rtsp_options[] = { > { "timeout", "set maximum timeout (in seconds) to wait for incoming > connections (-1 is infinite, imply flag listen) (deprecated, use > listen_timeout)", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, > INT_MIN, INT_MAX, DEC }, > { "stimeout", "set timeout (in microseconds) of socket TCP I/O > operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, > DEC }, > #else > -{ "timeout", "set timeout (in microseconds) of socket TCP I/O > operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, > DEC }, > +{ "timeout", "set timeout of socket TCP I/O operations", > OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 0}, 0, INT64_MAX, DEC }, > #endif > COMMON_OPTS(), > { "user_agent", "override User-Agent header", OFFSET(user_agent), > AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC }, > @@ -1818,7 +1818,8 @@ redirect: > /* open the tcp connection */ > ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, > host, port, > -"?timeout=%d", rt->stimeout); > +/* cast necessary until FF_API_OLD_RTSP_OPTIONS removed > */ > +"?timeout=%"PRId64, (int64_t)rt->stimeout); > if ((ret = ffurl_open_whitelist(&rt->rtsp_hd, tcpname, > AVIO_FLAG_READ_WRITE, > &s->interrupt_callback, NULL, s->protocol_whitelist, > s->protocol_blacklist, NULL)) < 0) { > err = ret; > diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h > index 9a7f366b39..1524962e1b 100644 > --- a/libavformat/rtsp.h > +++ b/libavformat/rtsp.h > @@ -395,7 +395,11 @@ typedef struct RTSPState { > /** > * timeout of socket i/o operations. > */ > +#if FF_API_OLD_RTSP_OPTIONS > int stimeout; > +#else > +int64_t stimeout; > +#endif > > /** > * Size of RTP packet reordering queue. On IRC I was asked to explain my NACK: The whole point of ff46124b0df17a1d35249e09ae8eae9a61f16e04 was to harmonize the timeout options with that of other protocols. In particular, http/tcp (the most used network protocol of them all) uses an AV_OPT_TYPE_INT "timeout" option, using microseconds. AV_OPT_TYPE_DURATION parses the time in seconds, so this is incompatible. It's incompatible on both CLI and API, because the API usually requires you to pass all options as string in AVDictionary entries (this is how dispatching general options to underlying protocols work, and it's impossible to access the options for sub protocols directly). Thus your change negates the whole point of the previous change, which is why I'm rejecting it. Reverting others patches after the discussion of it was done and everything finalized isn't exactly how team work works either. You have 2 choices: - change all timeout options to AV_OPT_TYPE_DURATION (after a deprecation period) - drop the patch Also, your patch message implies I pushed the previous patch too early. This is not true - I waited long enough, and because there was a flamewar, certainly it can't be claimed that you didn't notice it, or didn't have enough time to do whatever. Especially considering you replied to the thread on the same day as I posted the patch, and I pushed the patch over a week later. So I'm politely asking you to stop making such implications. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/hevc_idct : update test bit depth from 8 9 and 10 to 8 10 and 12
Hi, there. Is there any comment? I think i did the right fix. Yingming Fan > On 8 Mar 2018, at 4:17 PM, Hendrik Leppkes wrote: > > On Thu, Mar 8, 2018 at 9:16 AM, Paul B Mahol wrote: >> On 3/8/18, Yingming Fan wrote: >>> From: Yingming Fan >>> >>> --- >>> We have 8 10 and 12 SIMD codes, but previous checkasm hevc_idct only test 8 >>> and 10 bit depth. >>> >>> tests/checkasm/hevc_idct.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/tests/checkasm/hevc_idct.c b/tests/checkasm/hevc_idct.c >>> index eea712101d..c20111c2df 100644 >>> --- a/tests/checkasm/hevc_idct.c >>> +++ b/tests/checkasm/hevc_idct.c >>> @@ -87,7 +87,7 @@ void checkasm_check_hevc_idct(void) >>> { >>> int bit_depth; >>> >>> -for (bit_depth = 8; bit_depth <= 10; bit_depth++) { >>> +for (bit_depth = 8; bit_depth <= 12; bit_depth += 2) { >>> HEVCDSPContext h; >>> >>> ff_hevc_dsp_init(&h, bit_depth); >>> @@ -95,7 +95,7 @@ void checkasm_check_hevc_idct(void) >>> } >>> report("idct_dc"); >>> >>> -for (bit_depth = 8; bit_depth <= 10; bit_depth++) { >>> +for (bit_depth = 8; bit_depth <= 12; bit_depth += 2) { >>> HEVCDSPContext h; >>> >>> ff_hevc_dsp_init(&h, bit_depth); >> >> Isn't this dropping 9 case? > > It is, but we don't have any optimizations for 9 anyway, so there is > nothing to test. > > - Hendrik > ___ > 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] avcodec/arm/hevcdsp_sao : add NEON optimization for sao
Hi, there. I have already pushed a patch which add hevc_sao checkasm and patch was adopted. You can verify this optimization by using checkasm under arm device, `checkasm --test=hevc_sao --bench`. hevc_sao_band speed up ~2x, hevc_sao_edge speed up ~4x. Also passed FATE under arm platform. Yingming Fan > On 8 Mar 2018, at 3:03 PM, Yingming Fan wrote: > > From: Meng Wang > > Signed-off-by: Meng Wang > --- > As FFmpeg hevc decoder have no SAO neon optimization, we add sao_band and > sao_edge neon codes in this patch. > I have already submit a patch called 'checkasm/hevc_sao : add hevc_sao for > checkasm' several days ago. > Results below was printed by hevc_sao checkasm on an armv7 device Nexus 5. > From the results we can see: hevc_sao_band speed up ~2x, hevc_sao_edge speed > up ~4x. > Also test FATE under armv7 device and MacOS. > > hevc_sao_band_8x8_8_c: 804.9 > hevc_sao_band_8x8_8_neon: 452.4 > hevc_sao_band_16x16_8_c: 2638.1 > hevc_sao_band_16x16_8_neon: 1169.9 > hevc_sao_band_32x32_8_c: 9259.9 > hevc_sao_band_32x32_8_neon: 3956.1 > hevc_sao_band_48x48_8_c: 20344.6 > hevc_sao_band_48x48_8_neon: 8649.6 > hevc_sao_band_64x64_8_c: 35684.6 > hevc_sao_band_64x64_8_neon: 15213.1 > hevc_sao_edge_8x8_8_c: 1761.6 > hevc_sao_edge_8x8_8_neon: 414.6 > hevc_sao_edge_16x16_8_c: 6844.4 > hevc_sao_edge_16x16_8_neon: 1589.9 > hevc_sao_edge_32x32_8_c: 27156.4 > hevc_sao_edge_32x32_8_neon: 6116.6 > hevc_sao_edge_48x48_8_c: 60004.6 > hevc_sao_edge_48x48_8_neon: 13686.4 > hevc_sao_edge_64x64_8_c: 106708.1 > hevc_sao_edge_64x64_8_neon: 24240.1 > > libavcodec/arm/Makefile| 3 +- > libavcodec/arm/hevcdsp_init_neon.c | 63 + > libavcodec/arm/hevcdsp_sao_neon.S | 181 + > 3 files changed, 246 insertions(+), 1 deletion(-) > create mode 100644 libavcodec/arm/hevcdsp_sao_neon.S > > diff --git a/libavcodec/arm/Makefile b/libavcodec/arm/Makefile > index 1eeac5449e..2ee913e8a8 100644 > --- a/libavcodec/arm/Makefile > +++ b/libavcodec/arm/Makefile > @@ -136,7 +136,8 @@ NEON-OBJS-$(CONFIG_DCA_DECODER)+= > arm/synth_filter_neon.o > NEON-OBJS-$(CONFIG_HEVC_DECODER) += arm/hevcdsp_init_neon.o \ > arm/hevcdsp_deblock_neon.o\ > arm/hevcdsp_idct_neon.o \ > - arm/hevcdsp_qpel_neon.o > + arm/hevcdsp_qpel_neon.o \ > + arm/hevcdsp_sao_neon.o > NEON-OBJS-$(CONFIG_RV30_DECODER) += arm/rv34dsp_neon.o > NEON-OBJS-$(CONFIG_RV40_DECODER) += arm/rv34dsp_neon.o\ > arm/rv40dsp_neon.o > diff --git a/libavcodec/arm/hevcdsp_init_neon.c > b/libavcodec/arm/hevcdsp_init_neon.c > index a4628d2a93..3c480f12f8 100644 > --- a/libavcodec/arm/hevcdsp_init_neon.c > +++ b/libavcodec/arm/hevcdsp_init_neon.c > @@ -21,8 +21,16 @@ > #include "libavutil/attributes.h" > #include "libavutil/arm/cpu.h" > #include "libavcodec/hevcdsp.h" > +#include "libavcodec/avcodec.h" > #include "hevcdsp_arm.h" > > +void ff_hevc_sao_band_filter_neon_8_wrapper(uint8_t *_dst, uint8_t *_src, > + ptrdiff_t stride_dst, ptrdiff_t stride_src, > + int16_t *sao_offset_val, int > sao_left_class, > + int width, int height); > +void ff_hevc_sao_edge_filter_neon_8_wrapper(uint8_t *_dst, uint8_t *_src, > ptrdiff_t stride_dst, int16_t *sao_offset_val, > + int eo, int width, int height); > + > void ff_hevc_v_loop_filter_luma_neon(uint8_t *_pix, ptrdiff_t _stride, int > _beta, int *_tc, uint8_t *_no_p, uint8_t *_no_q); > void ff_hevc_h_loop_filter_luma_neon(uint8_t *_pix, ptrdiff_t _stride, int > _beta, int *_tc, uint8_t *_no_p, uint8_t *_no_q); > void ff_hevc_v_loop_filter_chroma_neon(uint8_t *_pix, ptrdiff_t _stride, int > *_tc, uint8_t *_no_p, uint8_t *_no_q); > @@ -142,6 +150,51 @@ QPEL_FUNC_UW(ff_hevc_put_qpel_uw_h3v2_neon_8); > QPEL_FUNC_UW(ff_hevc_put_qpel_uw_h3v3_neon_8); > #undef QPEL_FUNC_UW > > +void ff_hevc_sao_band_filter_neon_8(uint8_t *dst, uint8_t *src, ptrdiff_t > stride_dst, ptrdiff_t stride_src, int width, int height, int16_t > *offset_table); > + > +void ff_hevc_sao_band_filter_neon_8_wrapper(uint8_t *_dst, uint8_t *_src, > + ptrdiff_t stride_dst, ptrdiff_t stride_src, > + int16_t *sao_offset_val, int > sao_left_class, > + int width, int height) { > +uint8_t *dst = (uint8_t *)_dst; > +uint8_t *src = (uint8_t *)_src; > +int16_t offset_table[32] = {0}; > +int k; > + > +stride_dst /= sizeof(uint8_t); > +stride_src /= sizeof(uint8_t); > + > +for (k = 0; k < 4; k++) { > +offset_table[(k + sao_left_class) & 31] = sao_offset_val[k + 1]; > +}
Re: [FFmpeg-devel] GSoC
On Thu, Mar 8, 2018 at 8:57 AM, Mark Thompson wrote: > On 07/03/18 03:56, Dylan Fernando wrote: > > Thanks, it works now > > > > Would trying to implement an OpenCL version of vf_fade be a good idea > for a > > qualification task, or would it be a better idea to try a different > filter? > > That sounds like a sensible choice to me, though if you haven't written a > filter before you might find it helpful to write something simpler first to > understand how it fits together (for example: vflip, which has trivial > processing parts but still needs the surrounding boilerplate). > > - Mark > > (PS: be aware that top-posting is generally frowned upon on this mailing > list.) > > > > On Wed, Mar 7, 2018 at 1:20 AM, Mark Thompson wrote: > > > >> On 06/03/18 12:37, Dylan Fernando wrote: > >>> Hi, > >>> > >>> I am Dylan Fernando. I am a Computer Science student from Australia. I > am > >>> new to FFmpeg and I wish to apply for GSoC this year. > >>> I would like to do the Video filtering with OpenCL project and I have a > >> few > >>> questions. Would trying to implement an opencl version of vf_fade be a > >> good > >>> idea for the qualification task, or would I be better off using a > >> different > >>> filter? > >>> > >>> Also, I’m having a bit of trouble with running unsharp_opencl. I tried > >>> running: > >>> ffmpeg -hide_banner -nostats -v verbose -init_hw_device opencl=ocl:0.1 > >>> -filter_hw_device ocl -i space.mpg -filter_complex unsharp_opencl > >> output.mp4 > >>> > >>> but I got the error: > >>> [AVHWDeviceContext @ 0x7fdac050c700] 0.1: Apple / Intel(R) Iris(TM) > >>> Graphics 6100 > >>> [mpeg @ 0x7fdac3132600] max_analyze_duration 500 reached at 5005000 > >>> microseconds st:0 > >>> Input #0, mpeg, from 'space.mpg': > >>> Duration: 00:00:21.99, start: 0.387500, bitrate: 6108 kb/s > >>> Stream #0:0[0x1e0]: Video: mpeg2video (Main), 1 reference frame, > >>> yuv420p(tv, bt470bg, bottom first, left), 720x480 [SAR 8:9 DAR 4:3], > 6000 > >>> kb/s, 29.97 fps, 29.97 tbr, 90k tbn, 59.94 tbc > >>> Stream mapping: > >>> Stream #0:0 (mpeg2video) -> unsharp_opencl > >>> unsharp_opencl -> Stream #0:0 (mpeg4) > >>> Press [q] to stop, [?] for help > >>> [graph 0 input from stream 0:0 @ 0x7fdac0418800] w:720 h:480 > >> pixfmt:yuv420p > >>> tb:1/9 fr:3/1001 sar:8/9 sws_param:flags=2 > >>> [auto_scaler_0 @ 0x7fdac05232c0] w:iw h:ih flags:'bilinear' interl:0 > >>> [Parsed_unsharp_opencl_0 @ 0x7fdac0715a80] auto-inserting filter > >>> 'auto_scaler_0' between the filter 'graph 0 input from stream 0:0' and > >> the > >>> filter 'Parsed_unsharp_opencl_0' > >>> Impossible to convert between the formats supported by the filter > 'graph > >> 0 > >>> input from stream 0:0' and the filter 'auto_scaler_0' > >>> Error reinitializing filters! > >>> Failed to inject frame into filter network: Function not implemented > >>> Error while processing the decoded data for stream #0:0 > >>> Conversion failed! > >>> > >>> How do I correctly run unsharp_opencl? Should I be running it on a > >>> different video file? > >> > >> It's intended to be used in filter graphs where much of the activity is > >> already happening on the GPU, so the input and output are in the > >> AV_PIX_FMT_OPENCL format which contains GPU-side OpenCL images. > >> > >> If you want to use it standalone then you need hwupload and hwdownload > >> filters to move the frames between the CPU and GPU. For your example, > it > >> should work with: > >> > >> ffmpeg -init_hw_device opencl=ocl:0.1 -filter_hw_device ocl -i space.mpg > >> -filter_complex hwupload,unsharp_opencl,hwdownload output.mp4 > >> > >> (There are constraints on what formats can be used and therefore > suitable > >> files (or required format conversions), but I believe a normal yuv420p > >> video like this should work in all cases.) > >> > >> - Mark > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Thanks. How is AV_PIX_FMT_OPENCL formatted? When using read_imagef(), does xyzw correspond to RGBA respectively, or to YUV? Would I have to account for different formats? If so, how do I check the format of the input? Regards, Dylan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/mediacodecdec: add debug logging around hw buffer lifecycle
From: Aman Gupta Some Android devices are very finicky about how quicky output buffers are returned back to the decoder, especially when they are associated with a Surface. This commit adds a new counter that keeps track of exactly how many hw output buffers are being retained by the user, along with DEBUG level logging that makes it easy to track the lifecycle of these buffers. --- libavcodec/mediacodec.c | 7 --- libavcodec/mediacodecdec_common.c | 11 +++ libavcodec/mediacodecdec_common.h | 1 + 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/libavcodec/mediacodec.c b/libavcodec/mediacodec.c index 3ddd303c97..25491a01ec 100644 --- a/libavcodec/mediacodec.c +++ b/libavcodec/mediacodec.c @@ -92,9 +92,10 @@ int av_mediacodec_release_buffer(AVMediaCodecBuffer *buffer, int render) int released = atomic_fetch_add(&buffer->released, 1); if (!released && (ctx->delay_flush || buffer->serial == atomic_load(&ctx->serial))) { -av_log(ctx->avctx, AV_LOG_TRACE, - "Releasing output buffer %zd ts=%"PRId64" render=%d\n", - buffer->index, buffer->pts, render); +atomic_fetch_sub(&ctx->hw_buffers, 1); +av_log(ctx->avctx, AV_LOG_DEBUG, + "Releasing output buffer %zd (%p) ts=%"PRId64" with render=%d [%d pending]\n", + buffer->index, buffer, buffer->pts, render, atomic_load(&ctx->hw_buffers)); return ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, render); } diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c index 5064809cf6..f699609186 100644 --- a/libavcodec/mediacodecdec_common.c +++ b/libavcodec/mediacodecdec_common.c @@ -179,6 +179,10 @@ static void mediacodec_buffer_release(void *opaque, uint8_t *data) int released = atomic_load(&buffer->released); if (!released && (ctx->delay_flush || buffer->serial == atomic_load(&ctx->serial))) { +atomic_fetch_sub(&ctx->hw_buffers, 1); +av_log(ctx->avctx, AV_LOG_DEBUG, + "Releasing output buffer %zd (%p) ts=%"PRId64" on free() [%d pending]\n", + buffer->index, buffer, buffer->pts, atomic_load(&ctx->hw_buffers)); ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, 0); } @@ -246,6 +250,11 @@ FF_ENABLE_DEPRECATION_WARNINGS frame->data[3] = (uint8_t *)buffer; +atomic_fetch_add(&s->hw_buffers, 1); +av_log(avctx, AV_LOG_DEBUG, +"Wrapping output buffer %zd (%p) ts=%"PRId64" [%d pending]\n", +buffer->index, buffer, buffer->pts, atomic_load(&s->hw_buffers)); + return 0; fail: av_freep(buffer); @@ -429,6 +438,7 @@ static int mediacodec_dec_flush_codec(AVCodecContext *avctx, MediaCodecDecContex s->flushing = 0; s->eos = 0; atomic_fetch_add(&s->serial, 1); +atomic_init(&s->hw_buffers, 0); status = ff_AMediaCodec_flush(codec); if (status < 0) { @@ -454,6 +464,7 @@ int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s, s->avctx = avctx; atomic_init(&s->refcount, 1); +atomic_init(&s->hw_buffers, 0); atomic_init(&s->serial, 1); pix_fmt = ff_get_format(avctx, pix_fmts); diff --git a/libavcodec/mediacodecdec_common.h b/libavcodec/mediacodecdec_common.h index 3fd2412a65..8bfc67942d 100644 --- a/libavcodec/mediacodecdec_common.h +++ b/libavcodec/mediacodecdec_common.h @@ -38,6 +38,7 @@ typedef struct MediaCodecDecContext { AVCodecContext *avctx; atomic_int refcount; +atomic_int hw_buffers; char *codec_name; -- 2.14.3 (Apple Git-98) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add panorama filter
On 3/9/18, Paul B Mahol wrote: > On 3/9/18, Hazem Ashmawy wrote: >> On 3/9/18, Paul B Mahol wrote: >>> On 3/9/18, Hazem Ashmawy wrote: Add filter to convert between various panorama projections. It supports Equirectangular and Cubemaps (3x2 and 6x1 cubemap layouts). commit a8d80408bd9d99542cc29f30d7e6b00771846029 Author: Hazem Ashmawy Date: Thu Mar 8 10:09:36 2018 +0200 avfilter: add convertion to/from cubemap 6x1 Signed-off-by: Hazem Ashmawy commit f9f6a6cc0ceb1a4e749041658b7a441696b82588 Author: Paul B Mahol Date: Thu Dec 3 21:15:13 2015 +0100 avfilter: add panorama filter Signed-off-by: Paul B Mahol Signed-off-by: Hazem Ashmawy --- libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_panorama.c | 553 ++ 3 files changed, 555 insertions(+) create mode 100644 libavfilter/vf_panorama.c >>> >>> Here is partial change for converting 3x2 cubemap as given by latest >>> youtube videos to >>> equirectangular projection: >> >> Should I modify cube faces order to match the one specified here too? >> https://github.com/google/spatial-media/blob/master/docs/spherical-video-v2-rfc.md#semantics-3 > > Yes, The 3x2 cubemap projection should be what google use. > > There is also facebook own cubemap layout IIRC, but that one can came > later. > >> >>> >>> | diff --git a/libavfilter/vf_panorama.c b/libavfilter/vf_panorama.c >>> | index 7d9de5b62c..4d43fc0dd5 100644 >>> | --- a/libavfilter/vf_panorama.c >>> | +++ b/libavfilter/vf_panorama.c >>> | @@ -394,9 +394,9 @@ static int config_output(AVFilterLink *outlink) >>> | >>> | phi_threshold = atan2(1., 1. / cos(theta_norm)); >>> | if (phi > phi_threshold) { >>> | -face = DOWN; >>> | -} else if (phi < -phi_threshold) { >>> | face = TOP; >>> | +} else if (phi < -phi_threshold) { >>> | +face = DOWN; >>> | } else { >>> | ; >>> | } >>> | @@ -411,14 +411,14 @@ static int config_output(AVFilterLink *outlink) >>> | case RIGHT: >>> | locate(z, y, x, M_PI_2, rw, rh, &ox, &oy); >>> | break; >>> | -case TOP: >>> | -locate(y, z, x, M_PI, rw, rh, &ox, &oy); >>> | +case DOWN: >>> | +locate(y, z, x, M_PI_2, rw, rh, &ox, &oy); >>> | break; >>> | case BACK: >>> | -locate(x, y, z,-M_PI_2, rw, rh, &ox, &oy); >>> | +locate(x, y, z, 0, rw, rh, &ox, &oy); >>> | break; >>> | -case DOWN: >>> | -locate(y, x, z,-M_PI_2, rw, rh, &ox, &oy); >>> | +case TOP: >>> | +locate(y, x, z, M_PI , rw, rh, &ox, &oy); >>> | break; >>> | } >>> | >>> >>> Could you incorporate this? >>> And also update all other conversions so thay match with this one? >> Sure, I'll work on this. > > Good, you will also need to add frame side data support... but that can > wait. Actually frame side data support can not be used due certain libavfilter limitations, same ones as 3D frame side data patch for stereo3d filter. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Regarding the removal of hacks
Hi, I would like to raise a question regarding how the community feels about removing hacks which most likely were done to work around deficiencies of API clients. Silently. Some weeks ago someone asked on #ffmpeg why he wasn't getting the time scale (time base in MOV/ISOBMFF speak) he wanted out of movenc.c. So I looked into it: 1) https://git.videolan.org/?p=ffmpeg.git;a=commit;h=b02493e47668e66757b72a7163476e590edfea3a (2012) Some API clients didn't clearly set a time base correctly for VFR content - a hack was added in movenc.c to bump the time scale to at least 10k. 2) https://git.videolan.org/?p=ffmpeg.git;a=commit;h=7e570f027b9bb0b5504 ed443c70ceb654930859d (2013) Someone clearly didn't like this forced behavior so an AVOption was added to set a forced video time base for ALL video tracks in a mux. 3) https://git.videolan.org/?p=ffmpeg.git;a=commit;h=194be1f43ea391eb986732707435176e579265aa (2014) Anton switches the time base utilized from the encoder AVCodec to the AVStream, which most likely fixes the issue of output time base being incorrect as some encoders dislike weird (?) time bases (although how would you get VFR content through those if that was the reason is still beyond me). After this I told the user that this is a hack that's been added and that his timestamps are still exact (which they are, just that the time base is larger than it is). But as a developer, this just looks like the whole mess should be removed: 1) ffmpeg.c , for which the original hack possibly was added in 2012, now has an option to set output time base as far as I can remember? So the user of the tool can properly set a specific time base if required, just like with the AVOption. 2) This is just extra complexity/automated logic that can be hiding bugs in API clients, and we ended up having an additional AVOption. So, going forward, would patches to remove things like these be worth sending to the list? Best regards, Jan Ekström ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v4] avformat/pcm: decrease delay when reading PCM streams.
On 9 March 2018 at 23:33, Michael Niedermayer wrote: > On Fri, Mar 09, 2018 at 07:23:02PM +0100, Hendrik Leppkes wrote: > > On Fri, Mar 9, 2018 at 7:02 PM, Michael Niedermayer > > wrote: > > > On Fri, Mar 09, 2018 at 11:18:38AM +0100, Tomas Härdin wrote: > > >> On 2018-03-09 01:40, Michael Niedermayer wrote: > > >> >On Wed, Mar 07, 2018 at 03:30:37PM +0100, Philipp M. Scholl wrote: > > >> >> Here is the fourth version of the PCM patch with updated > testcases. > > >> >> > > >> >> The blocksize of the PCM decoder is hard-coded. This creates > > >> >>unnecessary delay when reading low-rate (<100Hz) streams. This > creates > > >> >>issues when multiplexing multiple streams, since other inputs are > only > > >> >>opened/read after a low-rate input block was completely read. > > >> >> > > >> >> This patch decreases the blocksize for low-rate inputs, so > > >> >>approximately a block is read every 40ms. This decreases the startup > > >> >>delay when multiplexing inputs with different rates. > > >> >> > > >> >>Signed-off-by: Philipp M. Scholl > > >> >>--- > > >> >> libavformat/pcm.c | 16 > > >> >> tests/ref/seek/lavf-alaw | 42 +- > > > >> >> tests/ref/seek/lavf-mulaw | 42 +- > > > >> >> 3 files changed, 54 insertions(+), 46 deletions(-) > > >> >> > > >> >>diff --git a/libavformat/pcm.c b/libavformat/pcm.c > > >> >>index 806f91b6b..1ea15a9e8 100644 > > >> >>--- a/libavformat/pcm.c > > >> >>+++ b/libavformat/pcm.c > > >> >>@@ -28,13 +28,21 @@ > > >> >> int ff_pcm_read_packet(AVFormatContext *s, AVPacket *pkt) > > >> >> { > > >> >>-int ret, size; > > >> >>+int ret, size = INT_MAX; > > >> >>+AVCodecParameters *par = s->streams[0]->codecpar; > > >> >>-size= RAW_SAMPLES*s->streams[0]->codecpar->block_align; > > >> >>-if (size <= 0) > > >> >>+if (par->block_align <= 0) > > >> >> return AVERROR(EINVAL); > > >> >>-ret= av_get_packet(s->pb, pkt, size); > > >> >>+/* > > >> >>+ * Compute read size to complete a read every 40ms. Clamp to > RAW_SAMPLES if > > >> >>+ * larger. Use power of two as readsize for I/O efficiency. > > >> >>+ */ > > >> >>+size = FFMAX(par->sample_rate/25, 1); > > >> >division is a bit slowish, and this is done per (small) packet. > > >> >Maybe a >>4 or >>5 could be used ? (this is a minor issue) > > >> > > >> It's not the 80's any more > > > > > > i do not think this comment is appropriate in a patch review. > > > > > > The goal is not to be different from fast code for the sake of > > > being different from fast code. Or? > > > > > > If a /25 is better than a >>4 or >>5 then it may make sense to > > > keep /25. But to me it seemed the value was completely arbitrary > > > so a /16 or /32 would do equally > > > > > > > Many common sample rates (ie. 22050, 44100, 88200, etc) don't evenly > > divide by 16 or 32 - they do however by 25. > > You would get oddly sized frames, which is not really advantageous. > > ok, that is an argument to favor /25 > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > What does censorship reveal? It reveals fear. -- Julian Assange > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > Besides, pretty much all compilers will optimize that to a multiply + a right shift. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Type mismatch in ADPCM
Hello, while working with ADPCM source code, I noticed that there is a type mismatch in ff_adpcm_afc_coeffs[2][16]. Inside libavcodec/adpcm_data.c it is declared as uint16_t: https://github.com/FFmpeg/FFmpeg/blob/d168e78effd170377ec57f67bca05c9f0de91bca/libavcodec/adpcm_data.c#L109 but into libavcodec/adpcm_data.h it is declared as int16_t: https://github.com/FFmpeg/FFmpeg/blob/d168e78effd170377ec57f67bca05c9f0de91bca/libavcodec/adpcm_data.h#L43 Those values are used as signed integer elsewhere. So, I would like to suggest to fix this declaration with attached patch. I noticed this thing because I compiled those sources with a more robust syntax check, by using C++ rather that plain C. At pratical level, nothing changed, except for the .h files that required to use the extern "C" keyword. I was wondering if you could evaulate the chance to add this feature to the include files. It could be done directly by using some #ifdef/#endif, or perhaps by doing something like this, somewhere in a common file: #ifdef __cplusplus #define FFMPEG_EXTERN_C_BEGIN extern "C" { #define FFMPEG_EXTERN_C_END } #else #define FFMPEG_EXTERN_C_BEGIN #define FFMPEG_EXTERN_C_END #endif and just add FFMPEG_EXTERN_C_BEGIN and FFMPEG_EXTERN_C_END to the top and to the bottom of each include file. You could take it as a suggestion, allowing compilation with both GCC and G++ may show you some defects you cannot see normally and give room to some improvements. Sincerely.diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c index cd3bbd33c2..2753bd852d 100644 --- a/libavcodec/adpcm.c +++ b/libavcodec/adpcm.c @@ -1254,7 +1254,7 @@ static int adpcm_decode_frame(AVCodecContext *avctx, void *data, int coeff1, coeff2; int shift; unsigned int channel; -uint16_t *samplesC; +int16_t *samplesC; int count = 0; int offsets[6]; diff --git a/libavcodec/adpcm_data.c b/libavcodec/adpcm_data.c index 4cce0a5857..e7ca5bc697 100644 --- a/libavcodec/adpcm_data.c +++ b/libavcodec/adpcm_data.c @@ -25,6 +25,8 @@ #include +#include "adpcm_data.h" + /* ff_adpcm_step_table[] and ff_adpcm_index_table[] are from the ADPCM reference source */ static const int8_t adpcm_index_table2[4] = { @@ -106,9 +108,9 @@ const int8_t ff_adpcm_yamaha_difflookup[] = { -1, -3, -5, -7, -9, -11, -13, -15 }; -const uint16_t ff_adpcm_afc_coeffs[2][16] = { -{ 0, 2048, 0, 1024, 4096, 3584, 3072, 4608, 4200, 4800, 5120, 2048, 1024, 64512, 64512, 63488 }, -{ 0, 0, 2048, 1024, 63488, 64000, 64512, 62976, 63288, 63236, 62464, 63488, 64512, 1024, 0, 0 } +const int16_t ff_adpcm_afc_coeffs[2][16] = { +{ 0, 2048, 0, 1024, 4096, 3584, 3072, 4608, 4200, 4800, 5120, 2048, 1024, -1024, -1024, -2048, }, +{ 0, 0, 2048, 1024, -2048, -1536, -1024, -2560, -2248, -2300, -3072, -2048, -1024, 1024, 0, 0, } }; const int16_t ff_adpcm_mtaf_stepsize[32][16] = { ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Regarding the removal of hacks
On 3/10/18, Jan Ekstroem wrote: > Hi, > > I would like to raise a question regarding how the community feels > about removing hacks which most likely were done to work around > deficiencies of API clients. Silently. > > Some weeks ago someone asked on #ffmpeg why he wasn't getting the time > scale (time base in MOV/ISOBMFF speak) he wanted out of movenc.c. So I > looked into it: > > 1) > https://git.videolan.org/?p=ffmpeg.git;a=commit;h=b02493e47668e66757b72a7163476e590edfea3a > (2012) Some API clients didn't clearly set a time base correctly for > VFR content - a hack was added in movenc.c to bump the time scale to > at least 10k. > 2) https://git.videolan.org/?p=ffmpeg.git;a=commit;h=7e570f027b9bb0b5504 > ed443c70ceb654930859d > (2013) Someone clearly didn't like this forced behavior so an > AVOption was added to set a forced video time base for ALL video > tracks in a mux. > 3) > https://git.videolan.org/?p=ffmpeg.git;a=commit;h=194be1f43ea391eb986732707435176e579265aa > (2014) Anton switches the time base utilized from the encoder > AVCodec to the AVStream, which most likely fixes the issue of output > time base being incorrect as some encoders dislike weird (?) time > bases (although how would you get VFR content through those if that > was the reason is still beyond me). > > After this I told the user that this is a hack that's been added and > that his timestamps are still exact (which they are, just that the > time base is larger than it is). But as a developer, this just looks > like the whole mess should be removed: > > 1) ffmpeg.c , for which the original hack possibly was added in 2012, > now has an option to set output time base as far as I can remember? So > the user of the tool can properly set a specific time base if > required, just like with the AVOption. > 2) This is just extra complexity/automated logic that can be hiding > bugs in API clients, and we ended up having an additional AVOption. > > So, going forward, would patches to remove things like these be worth > sending to the list? I'm all for exposing all hacks and removing all of them from the FFmpeg codebase. > > > Best regards, > Jan Ekstroem > ___ > 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] libavformat/movenc : Change MOV_TIMESCALE from 1000 to 600
On 9 Mar 2018, at 01:26, Carl Eugen Hoyos wrote: > This breaks fate, our regression testing suite (my mistake). > To download the test-suite: > $ make SAMPLES=fate-suite fate-rsync > $ make SAMPLES=fate-suite GEN=1 fate > This changes the values for fate, then commit again with: > $ git commit tests libavformat Thanks Carl, I hope the below is now correct. From 0c73563d06c05245ecf8bcaf64bc1c2809f375e4 Mon Sep 17 00:00:00 2001 From: mwjburton Date: Sat, 10 Mar 2018 14:33:02 + Subject: [PATCH] libavformat/movenc : Change MOV_TIMESCALE from 1000 to 600 Changing the MOV_TIMESCALE value from 1000 to 600 results in files that seek accurately in professional Quicktime software. This is due to the fact 600 is cleanly divisible by more frame rates than 1000, for example 24fps and 30fps. The Quicktime specification default is set to 600 for this reason. When set to 1000 seeking can be inaccurate for these fame rates. --- libavformat/movenc.h | 2 +- tests/ref/acodec/alac | 2 +- tests/ref/acodec/pcm-s16be | 2 +- tests/ref/acodec/pcm-s24be | 2 +- tests/ref/acodec/pcm-s32be | 2 +- tests/ref/acodec/pcm-s8| 2 +- tests/ref/fate/adtstoasc_ticket3715| 4 +-- tests/ref/fate/binsub-movtextenc | 2 +- tests/ref/fate/copy-psp| 6 ++-- tests/ref/fate/copy-trac236| 2 +- tests/ref/fate/copy-trac3074 | 2 +- tests/ref/fate/gaplessenc-itunes-to-ipod-aac | 8 ++--- tests/ref/fate/gaplessenc-pcm-to-mov-aac | 2 +- tests/ref/fate/movenc | 48 +- tests/ref/lavf-fate/mov_qtrle_mace6| 2 +- tests/ref/lavf/ismv| 6 ++-- tests/ref/lavf/mov | 14 tests/ref/vsynth/vsynth1-avui | 2 +- tests/ref/vsynth/vsynth1-dnxhd-1080i | 2 +- tests/ref/vsynth/vsynth1-dnxhd-1080i-10bit | 2 +- tests/ref/vsynth/vsynth1-dnxhd-1080i-colr | 2 +- tests/ref/vsynth/vsynth1-dnxhd-hr-hq-mov | 2 +- tests/ref/vsynth/vsynth1-dnxhd-hr-lb-mov | 2 +- tests/ref/vsynth/vsynth1-dnxhd-hr-sq-mov | 2 +- tests/ref/vsynth/vsynth1-mov-bgr24 | 2 +- tests/ref/vsynth/vsynth1-mov-bpp15 | 2 +- tests/ref/vsynth/vsynth1-mov-bpp16 | 2 +- tests/ref/vsynth/vsynth1-mpeg4 | 2 +- tests/ref/vsynth/vsynth1-prores| 2 +- tests/ref/vsynth/vsynth1-prores_ks | 2 +- tests/ref/vsynth/vsynth1-qtrle | 2 +- tests/ref/vsynth/vsynth1-qtrlegray | 2 +- tests/ref/vsynth/vsynth1-svq1 | 2 +- tests/ref/vsynth/vsynth1-vc2-420p | 2 +- tests/ref/vsynth/vsynth1-vc2-420p10| 2 +- tests/ref/vsynth/vsynth1-vc2-420p12| 2 +- tests/ref/vsynth/vsynth1-vc2-422p | 2 +- tests/ref/vsynth/vsynth1-vc2-422p10| 2 +- tests/ref/vsynth/vsynth1-vc2-422p12| 2 +- tests/ref/vsynth/vsynth1-vc2-444p | 2 +- tests/ref/vsynth/vsynth1-vc2-444p10| 2 +- tests/ref/vsynth/vsynth1-vc2-444p12| 2 +- tests/ref/vsynth/vsynth2-avui | 2 +- tests/ref/vsynth/vsynth2-dnxhd-1080i | 2 +- tests/ref/vsynth/vsynth2-dnxhd-1080i-10bit | 2 +- tests/ref/vsynth/vsynth2-dnxhd-1080i-colr | 2 +- tests/ref/vsynth/vsynth2-dnxhd-hr-hq-mov | 2 +- tests/ref/vsynth/vsynth2-dnxhd-hr-lb-mov | 2 +- tests/ref/vsynth/vsynth2-dnxhd-hr-sq-mov | 2 +- tests/ref/vsynth/vsynth2-mov-bgr24 | 2 +- tests/ref/vsynth/vsynth2-mov-bpp15 | 2 +- tests/ref/vsynth/vsynth2-mov-bpp16 | 2 +- tests/ref/vsynth/vsynth2-mpeg4 | 2 +- tests/ref/vsynth/vsynth2-prores| 2 +- tests/ref/vsynth/vsynth2-prores_ks | 2 +- tests/ref/vsynth/vsynth2-qtrle | 2 +- tests/ref/vsynth/vsynth2-qtrlegray | 2 +- tests/ref/vsynth/vsynth2-svq1 | 2 +- tests/ref/vsynth/vsynth2-vc2-420p | 2 +- tests/ref/vsynth/vsynth2-vc2-420p10| 2 +- tests/ref/vsynth/vsynth2-vc2-420p12| 2 +- tests/ref/vsynth/vsynth2-vc2-422p | 2 +- tests/ref/vsynth/vsynth2-vc2-422p10| 2 +- tests/ref/vsynth/vsynth2-vc2-422p12| 2 +- tests/ref/vsynth/vsynth2-vc2-444p | 2 +- tests/ref/vsynth/vsynth2-vc2-444p10| 2 +- tests/ref/vsynth/vsynth2-vc2-444p12| 2 +- tests/ref/vsynth/vsynth3-dnxhd-1080i-10bit | 2 +- tests/ref/vsynth/vsynth3-dnxhd-1080i-colr | 2 +- tests/ref/vsynth/vsynth3-dnxhd-hr-hq-mov | 2 +- tests/ref/vsynth/vsynth3-dnxhd-hr-lb-mov | 2 +- tests/ref/vsynth/vsynth3-dnxhd-hr-sq-mov |
Re: [FFmpeg-devel] Type mismatch in ADPCM
On Sat, 10 Mar 2018 15:02:56 +0100 (CET) Carlo Bramini wrote: > and just add FFMPEG_EXTERN_C_BEGIN and FFMPEG_EXTERN_C_END to the top and to > the bottom of each include file. You could take it as a suggestion, allowing > compilation with both GCC and G++ may show you some defects you cannot see > normally and give room to some improvements. FFmpeg can't be built with C ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vc2enc: replace quantization LUT with a smaller division LUT
On 28 February 2018 at 23:38, Michael Niedermayer wrote: > On Tue, Feb 27, 2018 at 11:12:33PM +, Rostislav Pehlivanov wrote: > > This commit replaces the huge and impractical LUT which converted coeffs > > and a quantizer to bits to encode and instead uses a standard > multiplication > > and a shift to replace the division and then codes the values using the > > regular golomb coding functions. > > I was unable to see a performance difference on my machine but perhaps > > someone else here can test. In any case, its better than the old one if > > only because its smaller and less intrusive. > > > > Signed-off-by: Rostislav Pehlivanov > > --- > > libavcodec/vc2enc.c | 118 ++ > -- > > 1 file changed, 31 insertions(+), 87 deletions(-) > [...] > > > @@ -557,7 +521,7 @@ static void encode_picture_start(VC2EncContext *s) > > encode_wavelet_transform(s); > > } > > > > -#define QUANT(c, qf) (((c) << 2)/(qf)) > > +#define QUANT(c, mul, add, shift) ((mul * c + add) >> shift) > > This needs more () otherwise this will misbehave with many > expressions, for example a+b in mul would be a+b*c not (a+b)*c > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Many that live deserve death. And some that die deserve life. Can you give > it to them? Then do not be too eager to deal out death in judgement. For > even the very wise cannot see all ends. -- Gandalf > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > Fate passes, pushed with more brackets. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Type mismatch in ADPCM
On Sat, 10 Mar 2018 15:02:56 +0100 (CET) Carlo Bramini wrote: > and just add FFMPEG_EXTERN_C_BEGIN and FFMPEG_EXTERN_C_END to the top and to > the bottom of each include file. You could take it as a suggestion, allowing > compilation with both GCC and G++ may show you some defects you cannot see > normally and give room to some improvements. FFmpeg can't be built with C++ because it uses some C99 only features. Besides, having to add all that casting to void*->structtype* would be annoying. Ignore my previous mail, shitty mail client maps + to "send mail" or something. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v5] avformat/pcm: decrease delay when reading PCM streams.
Thanks for the discussion. Here's the next version, now with /25 and removed ff_log2(). The blocksize of the PCM decoder is hard-coded. This creates unnecessary delay when reading low-rate (<100Hz) streams. This creates issues when multiplexing multiple streams, since other inputs are only opened/read after a low-rate input block was completely read. This patch decreases the blocksize for low-rate inputs, so approximately a block is read every 40ms. This decreases the startup delay when multiplexing inputs with different rates. Signed-off-by: Philipp M. Scholl --- libavformat/pcm.c | 13 ++--- tests/ref/seek/lavf-alaw | 42 +- tests/ref/seek/lavf-mulaw | 42 +- 3 files changed, 52 insertions(+), 45 deletions(-) diff --git a/libavformat/pcm.c b/libavformat/pcm.c index 806f91b6b..767bbd045 100644 --- a/libavformat/pcm.c +++ b/libavformat/pcm.c @@ -28,13 +28,20 @@ int ff_pcm_read_packet(AVFormatContext *s, AVPacket *pkt) { +AVCodecParameters *par = s->streams[0]->codecpar; int ret, size; -size= RAW_SAMPLES*s->streams[0]->codecpar->block_align; -if (size <= 0) +if (par->block_align <= 0) return AVERROR(EINVAL); -ret= av_get_packet(s->pb, pkt, size); +/* + * Compute read size to complete a read every 62ms. + * Clamp to RAW_SAMPLES if larger. + */ +size = FFMAX(par->sample_rate/25, 1); +size = FFMIN(size, RAW_SAMPLES) * par->block_align; + +ret = av_get_packet(s->pb, pkt, size); pkt->flags &= ~AV_PKT_FLAG_CORRUPT; pkt->stream_index = 0; diff --git a/tests/ref/seek/lavf-alaw b/tests/ref/seek/lavf-alaw index 4b1f8fbc0..8d517fa2b 100644 --- a/tests/ref/seek/lavf-alaw +++ b/tests/ref/seek/lavf-alaw @@ -1,53 +1,53 @@ -ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 1024 +ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 882 ret: 0 st:-1 flags:0 ts:-1.00 -ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 1024 +ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 882 ret: 0 st:-1 flags:1 ts: 1.894167 -ret: 0 st: 0 flags:1 dts: 1.894150 pts: 1.894150 pos: 41766 size: 1024 +ret: 0 st: 0 flags:1 dts: 1.894150 pts: 1.894150 pos: 41766 size: 882 ret: 0 st: 0 flags:0 ts: 0.788345 -ret: 0 st: 0 flags:1 dts: 0.788345 pts: 0.788345 pos: 17383 size: 1024 +ret: 0 st: 0 flags:1 dts: 0.788345 pts: 0.788345 pos: 17383 size: 882 ret: 0 st: 0 flags:1 ts:-0.317506 -ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 1024 +ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 882 ret: 0 st:-1 flags:0 ts: 2.576668 ret:-EOF ret: 0 st:-1 flags:1 ts: 1.470835 -ret: 0 st: 0 flags:1 dts: 1.470839 pts: 1.470839 pos: 32432 size: 1024 +ret: 0 st: 0 flags:1 dts: 1.470839 pts: 1.470839 pos: 32432 size: 882 ret: 0 st: 0 flags:0 ts: 0.364989 -ret: 0 st: 0 flags:1 dts: 0.364989 pts: 0.364989 pos: 8048 size: 1024 +ret: 0 st: 0 flags:1 dts: 0.364989 pts: 0.364989 pos: 8048 size: 882 ret: 0 st: 0 flags:1 ts:-0.740816 -ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 1024 +ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 882 ret: 0 st:-1 flags:0 ts: 2.153336 ret:-EOF ret: 0 st:-1 flags:1 ts: 1.047503 -ret: 0 st: 0 flags:1 dts: 1.047483 pts: 1.047483 pos: 23097 size: 1024 +ret: 0 st: 0 flags:1 dts: 1.047483 pts: 1.047483 pos: 23097 size: 882 ret: 0 st: 0 flags:0 ts:-0.058322 -ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 1024 +ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 882 ret: 0 st: 0 flags:1 ts: 2.835828 ret:-EOF ret: 0 st:-1 flags:0 ts: 1.730004 -ret: 0 st: 0 flags:1 dts: 1.730023 pts: 1.730023 pos: 38147 size: 1024 +ret: 0 st: 0 flags:1 dts: 1.730023 pts: 1.730023 pos: 38147 size: 882 ret: 0 st:-1 flags:1 ts: 0.624171 -ret: 0 st: 0 flags:1 dts: 0.624172 pts: 0.624172 pos: 13763 size: 1024 +ret: 0 st: 0 flags:1 dts: 0.624172 pts: 0.624172 pos: 13763 size: 882 ret: 0 st: 0 flags:0 ts:-0.481678 -ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 1024 +ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos: 0 size: 882 ret: 0 st: 0 flags:1 ts: 2.412517 ret:-EOF ret: 0 st:-1 flags:0 ts: 1.306672 -ret: 0 st: 0 flags:1 dts: 1.306667 pts: 1.306667 pos: 28812 size: 1024 +ret: 0 st: 0 flags:1 dts: 1.306667 pts: 1.306667 pos: 28812 size: 882 ret: 0 st:-1 flags:1 ts: 0.200839 -ret: 0 st:
Re: [FFmpeg-devel] [PATCH v5] avformat/pcm: decrease delay when reading PCM streams.
On 3/10/18, Philipp M. Scholl wrote: > Thanks for the discussion. Here's the next version, now with /25 and > removed > ff_log2(). > > The blocksize of the PCM decoder is hard-coded. This creates > unnecessary delay when reading low-rate (<100Hz) streams. This creates > issues when multiplexing multiple streams, since other inputs are only > opened/read after a low-rate input block was completely read. > > This patch decreases the blocksize for low-rate inputs, so > approximately a block is read every 40ms. This decreases the startup > delay when multiplexing inputs with different rates. > > Signed-off-by: Philipp M. Scholl > --- > libavformat/pcm.c | 13 ++--- > tests/ref/seek/lavf-alaw | 42 +- > tests/ref/seek/lavf-mulaw | 42 +- > 3 files changed, 52 insertions(+), 45 deletions(-) > OK ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 6/7] avcodec/ffv1: support of more pix_fmt
On Fri, Mar 09, 2018 at 08:56:56PM +0100, Jerome Martinez wrote: > On 09/03/2018 18:30, Paul B Mahol wrote: > >On 3/7/18, Jerome Martinez wrote: > >>With some sources having specific pix_fmt (9/10/12/14 bit), the source > >>is padded to 16-bit when the pix_fmt is not supported natively by the > >>FFV1 encoder. > >>Nothing is lost ("cutting" to the source bitdepth permits to retrieve > >>the exact source), but the source bitdepth is not indicated so it is not > >>possible to retrieve the exact source without having the source bitdepth > >>by another channel. It also makes FATE tests (framemd5 comparison) a bit > >>more complicated (bitdepth reduction). > >> > >>This patch adds native support of the pix_fmt being padded, so there is > >>no more padding and the FFV1 bitdepth is same as source bitdepth. > >> > >Trailing whitespaces are not allowed in FFmpeg codebase. > > Was not intended. > Updated patch attached. > ffv1dec.c | 14 +- > ffv1enc.c | 13 - > 2 files changed, 25 insertions(+), 2 deletions(-) > 5f129d841fc5f96067f48697c156a04b3cacf33c > 0001-avcodec-ffv1-support-of-more-pix_fmt.patch > From 6e9a50ba90c3264c26b4e775e0618ee1702812a8 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= > Date: Wed, 7 Mar 2018 11:19:03 +0100 > Subject: [PATCH] avcodec/ffv1: support of more pix_fmt > > Without direct support of such pix_fmt, content is padded to 16-bit > and it is not possible to know that the source file was with a smaller bit > depth > so framemd5 is different > --- > libavcodec/ffv1dec.c | 14 +- > libavcodec/ffv1enc.c | 13 - > 2 files changed, 25 insertions(+), 2 deletions(-) will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/extract_extradata: don't allocate more space than needed when removing NALUs in h264/hevc
On 3/9/2018 2:40 PM, James Almer wrote: > Signed-off-by: James Almer > --- > libavcodec/extract_extradata_bsf.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/extract_extradata_bsf.c > b/libavcodec/extract_extradata_bsf.c > index fbfd12aeef..4e2d601742 100644 > --- a/libavcodec/extract_extradata_bsf.c > +++ b/libavcodec/extract_extradata_bsf.c > @@ -62,7 +62,7 @@ static int extract_extradata_h2645(AVBSFContext *ctx, > AVPacket *pkt, > ExtractExtradataContext *s = ctx->priv_data; > > H2645Packet h2645_pkt = { 0 }; > -int extradata_size = 0; > +int extradata_size = 0, filtered_size = 0; > const int *extradata_nal_types; > int nb_extradata_nal_types; > int i, has_sps = 0, has_vps = 0, ret = 0; > @@ -90,6 +90,8 @@ static int extract_extradata_h2645(AVBSFContext *ctx, > AVPacket *pkt, > } else { > if (nal->type == H264_NAL_SPS) has_sps = 1; > } > +} else if (s->remove) { > +filtered_size += nal->raw_size + 3; > } > } > > @@ -100,11 +102,13 @@ static int extract_extradata_h2645(AVBSFContext *ctx, > AVPacket *pkt, > uint8_t *extradata, *filtered_data; > > if (s->remove) { > -filtered_buf = av_buffer_alloc(pkt->size + > AV_INPUT_BUFFER_PADDING_SIZE); > +filtered_buf = av_buffer_alloc(filtered_size + > AV_INPUT_BUFFER_PADDING_SIZE); > if (!filtered_buf) { > ret = AVERROR(ENOMEM); > goto fail; > } > +memset(filtered_buf->data + filtered_size, 0, > AV_INPUT_BUFFER_PADDING_SIZE); > + > filtered_data = filtered_buf->data; > } > > @@ -137,9 +141,7 @@ static int extract_extradata_h2645(AVBSFContext *ctx, > AVPacket *pkt, > av_buffer_unref(&pkt->buf); > pkt->buf = filtered_buf; > pkt->data = filtered_buf->data; > -pkt->size = filtered_data - filtered_buf->data; > - > -memset(pkt->data + pkt->size, 0, AV_INPUT_BUFFER_PADDING_SIZE); > +pkt->size = filtered_size; > } > } Will push this later today. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Use the correct offset to shift timestamp when seeking.
On Fri, Mar 09, 2018 at 01:30:21PM -0800, Sasi Inguva wrote: > Fixes seek for files with empty edits and files with negative ctts > (dts_shift > 0). Added fate samples and tests. > > Signed-off-by: Sasi Inguva > --- > libavformat/isom.h | 1 + > libavformat/mov.c| 27 +++-- > tests/fate/seek.mak | 6 + > tests/ref/seek/empty-edit-mp4| 134 +++ > tests/ref/seek/test-iibbibb-mp4 | 122 + > tests/ref/seek/test-iibbibb-neg-ctts-mp4 | 122 + > 6 files changed, 401 insertions(+), 11 deletions(-) > create mode 100644 tests/ref/seek/empty-edit-mp4 > create mode 100644 tests/ref/seek/test-iibbibb-mp4 > create mode 100644 tests/ref/seek/test-iibbibb-neg-ctts-mp4 will apply thanks [...] -- 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: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Regarding the removal of hacks
On Sat, Mar 10, 2018 at 03:08:16PM +0200, Jan Ekström wrote: > Hi, > > I would like to raise a question regarding how the community feels > about removing hacks which most likely were done to work around > deficiencies of API clients. Silently. Iam in favor of removing hacks and replacing them by better solutions. To do this its important to first understand why a hack was needed and what it does. Otherwise no correct or better implementation is possible and removial would just add back a bug. > > Some weeks ago someone asked on #ffmpeg why he wasn't getting the time > scale (time base in MOV/ISOBMFF speak) he wanted out of movenc.c. So I > looked into it: > > 1) > https://git.videolan.org/?p=ffmpeg.git;a=commit;h=b02493e47668e66757b72a7163476e590edfea3a > (2012) Some API clients didn't clearly set a time base correctly for > VFR content - a hack was added in movenc.c to bump the time scale to > at least 10k. Iam not sure this was related to VFR as you write here. I think what this was about was A/V desync on more than VFR videos to illustrate, consider that if you create a slide show with 1fps you get upto 1 second desync if you are unlucky. the same happens with normal frame rates in CFR video. That is this fixed the case where a audio and video sample needed to be synced exactly and not by upto 1/30 only (for 30fps video) This problem was IIRC interlinked with the need to shift initial negative DTS into the zero or positive. So even if an application supplied exact timestamps in a low timebase like 1/30. after the shift to get DTS after 0 they would no longer be where they where before when the timebases where not accurate enough. So in from the applications point of view 1/30 could be fully exact and good enough yet because dts where negative and needed to be shifted a more precisse timebase was needed to do this without introducing AV desync. This issue may have been fixed with edit lists > 2) https://git.videolan.org/?p=ffmpeg.git;a=commit;h=7e570f027b9bb0b5504 > ed443c70ceb654930859d > (2013) Someone clearly didn't like this forced behavior so an > AVOption was added to set a forced video time base for ALL video > tracks in a mux. > 3) > https://git.videolan.org/?p=ffmpeg.git;a=commit;h=194be1f43ea391eb986732707435176e579265aa > (2014) Anton switches the time base utilized from the encoder > AVCodec to the AVStream, which most likely fixes the issue of output > time base being incorrect as some encoders dislike weird (?) time > bases (although how would you get VFR content through those if that > was the reason is still beyond me). > > After this I told the user that this is a hack that's been added and > that his timestamps are still exact (which they are, just that the > time base is larger than it is). But as a developer, this just looks > like the whole mess should be removed: > > 1) ffmpeg.c , for which the original hack possibly was added in 2012, > now has an option to set output time base as far as I can remember? So > the user of the tool can properly set a specific time base if > required, just like with the AVOption. > 2) This is just extra complexity/automated logic that can be hiding > bugs in API clients, and we ended up having an additional AVOption. > > So, going forward, would patches to remove things like these be worth > sending to the list? yes, but in the more general case it could require a bit of care and could in some cases be alot more work than it initially looks like. It also would likely often require quite a bit of testing and figuring out what the "to be removed" code exactly was needed for. That is testing, code archiology, debuging, and where hacks still are needed design and implementation of better solutions. thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/mpeg4videodec: Use more specific error codes
Signed-off-by: Michael Niedermayer --- libavcodec/mpeg4videodec.c | 100 +++-- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c index 19210d97fe..1357b357a8 100644 --- a/libavcodec/mpeg4videodec.c +++ b/libavcodec/mpeg4videodec.c @@ -448,7 +448,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext *ctx) /* is there enough space left for a video packet + header */ if (get_bits_count(&s->gb) > s->gb.size_in_bits - 20) -return -1; +return AVERROR_INVALIDDATA; for (len = 0; len < 32; len++) if (get_bits1(&s->gb)) @@ -456,7 +456,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext *ctx) if (len != ff_mpeg4_get_video_packet_prefix_length(s)) { av_log(s->avctx, AV_LOG_ERROR, "marker does not match f_code\n"); -return -1; +return AVERROR_INVALIDDATA; } if (ctx->shape != RECT_SHAPE) { @@ -468,7 +468,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext *ctx) if (mb_num >= s->mb_num || !mb_num) { av_log(s->avctx, AV_LOG_ERROR, "illegal mb_num in video packet (%d %d) \n", mb_num, s->mb_num); -return -1; +return AVERROR_INVALIDDATA; } s->mb_x = mb_num % s->mb_width; @@ -597,7 +597,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, int n, int *dir_ptr) if (code < 0 || code > 9 /* && s->nbit < 9 */) { av_log(s->avctx, AV_LOG_ERROR, "illegal dc vlc\n"); -return -1; +return AVERROR_INVALIDDATA; } if (code == 0) { @@ -620,7 +620,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, int n, int *dir_ptr) if (get_bits1(&s->gb) == 0) { /* marker */ if (s->avctx->err_recognition & (AV_EF_BITSTREAM|AV_EF_COMPLIANT)) { av_log(s->avctx, AV_LOG_ERROR, "dc marker bit missing\n"); -return -1; +return AVERROR_INVALIDDATA; } } } @@ -664,7 +664,7 @@ static int mpeg4_decode_partition_a(Mpeg4DecContext *ctx) if (cbpc < 0) { av_log(s->avctx, AV_LOG_ERROR, "mcbpc corrupted at %d %d\n", s->mb_x, s->mb_y); -return -1; +return cbpc; } } while (cbpc == 8); @@ -684,7 +684,7 @@ static int mpeg4_decode_partition_a(Mpeg4DecContext *ctx) if (dc < 0) { av_log(s->avctx, AV_LOG_ERROR, "DC corrupted at %d %d\n", s->mb_x, s->mb_y); -return -1; +return dc; } dir <<= 1; if (dc_pred_dir) @@ -736,7 +736,7 @@ try_again: if (cbpc < 0) { av_log(s->avctx, AV_LOG_ERROR, "mcbpc corrupted at %d %d\n", s->mb_x, s->mb_y); -return -1; +return cbpc; } if (cbpc == 20) goto try_again; @@ -774,11 +774,11 @@ try_again: if (!s->mcsel) { mx = ff_h263_decode_motion(s, pred_x, s->f_code); if (mx >= 0x) -return -1; +return AVERROR_INVALIDDATA; my = ff_h263_decode_motion(s, pred_y, s->f_code); if (my >= 0x) -return -1; +return AVERROR_INVALIDDATA; s->current_picture.mb_type[xy] = MB_TYPE_16x16 | MB_TYPE_L0; } else { @@ -805,11 +805,11 @@ try_again: int16_t *mot_val = ff_h263_pred_motion(s, i, 0, &pred_x, &pred_y); mx = ff_h263_decode_motion(s, pred_x, s->f_code); if (mx >= 0x) -return -1; +return AVERROR_INVALIDDATA; my = ff_h263_decode_motion(s, pred_y, s->f_code); if (my >= 0x) -return -1; +return AVERROR_INVALIDDATA; mot_val[0] = mx; mot_val[1] = my; } @@ -850,7 +850,7 @@ static int mpeg4_decode_partition_b(MpegEncContext *s, int mb_count) if (cbpy < 0) { av_log(s->avctx, AV_LOG_ERROR, "cbpy corrupted at %d %d\n", s->mb_x, s->mb_y); -return -1; +return cb
[FFmpeg-devel] [PATCH] doc/utils: document the "ms" and "us" suffixes for durations
These suffixes were recently introduced in 61c972384d311508d07f9360d196909e27195655 and completed in 8218249f1f04de65904f58519bde21948e5a0783. Signed-off-by: Moritz Barsnick --- I chose not to document the suffixes in the section describing the HH:MM:SS.mmm syntax, even though they work there (with expected, but quite difficult to explain effects). doc/utils.texi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/utils.texi b/doc/utils.texi index d55dd315c3..a094ee151c 100644 --- a/doc/utils.texi +++ b/doc/utils.texi @@ -110,11 +110,12 @@ maximum of 2 digits. The @var{m} at the end expresses decimal value for @emph{or} @example -[-]@var{S}+[.@var{m}...] +[-]@var{S}+[.@var{m}...][ms|us] @end example @var{S} expresses the number of seconds, with the optional decimal part -@var{m}. +@var{m}. The optional literal suffixes @samp{ms} or @samp{us} indicate to +interpret the value as milliseconds or microseconds, respectively. In both expressions, the optional @samp{-} indicates negative duration. -- 2.14.3 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/mpeg4videodec: Use more specific error codes
On 3/10/2018 2:34 PM, Michael Niedermayer wrote: > Signed-off-by: Michael Niedermayer > --- > libavcodec/mpeg4videodec.c | 100 > +++-- > 1 file changed, 51 insertions(+), 49 deletions(-) > > diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c > index 19210d97fe..1357b357a8 100644 > --- a/libavcodec/mpeg4videodec.c > +++ b/libavcodec/mpeg4videodec.c > @@ -448,7 +448,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext > *ctx) > > /* is there enough space left for a video packet + header */ > if (get_bits_count(&s->gb) > s->gb.size_in_bits - 20) > -return -1; > +return AVERROR_INVALIDDATA; > > for (len = 0; len < 32; len++) > if (get_bits1(&s->gb)) > @@ -456,7 +456,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext > *ctx) > > if (len != ff_mpeg4_get_video_packet_prefix_length(s)) { > av_log(s->avctx, AV_LOG_ERROR, "marker does not match f_code\n"); > -return -1; > +return AVERROR_INVALIDDATA; > } > > if (ctx->shape != RECT_SHAPE) { > @@ -468,7 +468,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext > *ctx) > if (mb_num >= s->mb_num || !mb_num) { > av_log(s->avctx, AV_LOG_ERROR, > "illegal mb_num in video packet (%d %d) \n", mb_num, > s->mb_num); > -return -1; > +return AVERROR_INVALIDDATA; > } > > s->mb_x = mb_num % s->mb_width; > @@ -597,7 +597,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, int > n, int *dir_ptr) > > if (code < 0 || code > 9 /* && s->nbit < 9 */) { > av_log(s->avctx, AV_LOG_ERROR, "illegal dc vlc\n"); > -return -1; > +return AVERROR_INVALIDDATA; > } > > if (code == 0) { > @@ -620,7 +620,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, int > n, int *dir_ptr) > if (get_bits1(&s->gb) == 0) { /* marker */ > if (s->avctx->err_recognition & > (AV_EF_BITSTREAM|AV_EF_COMPLIANT)) { > av_log(s->avctx, AV_LOG_ERROR, "dc marker bit > missing\n"); > -return -1; > +return AVERROR_INVALIDDATA; > } > } > } > @@ -664,7 +664,7 @@ static int mpeg4_decode_partition_a(Mpeg4DecContext *ctx) > if (cbpc < 0) { > av_log(s->avctx, AV_LOG_ERROR, > "mcbpc corrupted at %d %d\n", s->mb_x, > s->mb_y); > -return -1; > +return cbpc; get_vlc2() seems to return -1 on error, so nothing really changes with this. Same with every other similar call, just hardcode AVERROR_INVALIDDATA as well instead. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/rtsp: fix the type of the timeout option.
A timeout is a duration of time, therefore the correct type for it is AV_OPT_TYPE_DURATION. It has the benefit of offering a better user interface, with units specification. Unfortunately, ff46124b0df17a1d35249e09ae8eae9a61f16e04 was pushed before that mistake could be corrected. Signed-off-by: Nicolas George --- libavformat/rtsp.c | 5 +++-- libavformat/rtsp.h | 4 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index ceb770a3a4..1fbdcfcedd 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -98,7 +98,7 @@ const AVOption ff_rtsp_options[] = { { "timeout", "set maximum timeout (in seconds) to wait for incoming connections (-1 is infinite, imply flag listen) (deprecated, use listen_timeout)", OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, DEC }, { "stimeout", "set timeout (in microseconds) of socket TCP I/O operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC }, #else -{ "timeout", "set timeout (in microseconds) of socket TCP I/O operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN, INT_MAX, DEC }, +{ "timeout", "set timeout of socket TCP I/O operations", OFFSET(stimeout), AV_OPT_TYPE_DURATION, {.i64 = 0}, 0, INT64_MAX, DEC }, #endif COMMON_OPTS(), { "user_agent", "override User-Agent header", OFFSET(user_agent), AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT}, 0, 0, DEC }, @@ -1818,7 +1818,8 @@ redirect: /* open the tcp connection */ ff_url_join(tcpname, sizeof(tcpname), lower_rtsp_proto, NULL, host, port, -"?timeout=%d", rt->stimeout); +/* cast necessary until FF_API_OLD_RTSP_OPTIONS removed */ +"?timeout=%"PRId64, (int64_t)rt->stimeout); if ((ret = ffurl_open_whitelist(&rt->rtsp_hd, tcpname, AVIO_FLAG_READ_WRITE, &s->interrupt_callback, NULL, s->protocol_whitelist, s->protocol_blacklist, NULL)) < 0) { err = ret; diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h index 9a7f366b39..1524962e1b 100644 --- a/libavformat/rtsp.h +++ b/libavformat/rtsp.h @@ -395,7 +395,11 @@ typedef struct RTSPState { /** * timeout of socket i/o operations. */ +#if FF_API_OLD_RTSP_OPTIONS int stimeout; +#else +int64_t stimeout; +#endif /** * Size of RTP packet reordering queue. -- 2.16.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Type mismatch in ADPCM
2018-03-10 15:02 GMT+01:00 Carlo Bramini : > I noticed this thing because I compiled those sources with a more robust > syntax check, by using C++ rather that plain C. At pratical level, nothing > changed, except for the .h files that required to use the extern "C" keyword. > I was wondering if you could evaulate the chance to add this feature to the > include files. > It could be done directly by using some #ifdef/#endif, or perhaps by doing > something like this, somewhere in a common file: > > #ifdef __cplusplus > #define FFMPEG_EXTERN_C_BEGIN extern "C" { > #define FFMPEG_EXTERN_C_END } > #else > #define FFMPEG_EXTERN_C_BEGIN > #define FFMPEG_EXTERN_C_END > #endif In addition to what was said: We cannot commit above because we would not test it and we cannot guarantee that it will always work: We provide C headers, if you want to use them in a C++ project, it is your responsibility to make sure it works, not ours. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]lavfi/deshake: Check alignment before calling asm init function
Hi! Attached patch fixes ticket #7078 for me. Please comment, Carl Eugen From 75ead282c3aa3c214d37e766690e2edd037307c0 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos Date: Sat, 10 Mar 2018 20:46:21 +0100 Subject: [PATCH] lavfi/deshake: Check alignment before calling asm init function. Do this for every frame to make sure dynamic filters do not cause crashes. Fixes ticket #7078. --- libavfilter/vf_deshake.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c index fb4eb35..75e9990 100644 --- a/libavfilter/vf_deshake.c +++ b/libavfilter/vf_deshake.c @@ -342,10 +342,6 @@ static av_cold int init(AVFilterContext *ctx) { DeshakeContext *deshake = ctx->priv; -deshake->sad = av_pixelutils_get_sad_fn(4, 4, 1, deshake); // 16x16, 2nd source unaligned -if (!deshake->sad) -return AVERROR(EINVAL); - deshake->refcount = 20; // XXX: add to options? deshake->blocksize /= 2; deshake->blocksize = av_clip(deshake->blocksize, 4, 128); @@ -432,6 +428,10 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) } av_frame_copy_props(out, in); +deshake->sad = av_pixelutils_get_sad_fn(4, 4, !((unsigned long)in->data[0] & 15), deshake); // 16x16, 2nd source unaligned +if (!deshake->sad) +return AVERROR(EINVAL); + if (deshake->cx < 0 || deshake->cy < 0 || deshake->cw < 0 || deshake->ch < 0) { // Find the most likely global motion for the current frame find_motion(deshake, (deshake->ref == NULL) ? in->data[0] : deshake->ref->data[0], in->data[0], link->w, link->h, in->linesize[0], &t); -- 1.8.4.5 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/h2645_parse: Replace RNXYA by RNXY in ff_h2645_extract_rbsp()
Fixes misaligned accesses Found-by: Matt Wolenetz Signed-off-by: Michael Niedermayer --- libavcodec/h2645_parse.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c index e6c40381b0..dbf2435677 100644 --- a/libavcodec/h2645_parse.c +++ b/libavcodec/h2645_parse.c @@ -53,8 +53,8 @@ int ff_h2645_extract_rbsp(const uint8_t *src, int length, i++ #if HAVE_FAST_64BIT for (i = 0; i + 1 < length; i += 9) { -if (!((~AV_RN64A(src + i) & - (AV_RN64A(src + i) - 0x0100010001000101ULL)) & +if (!((~AV_RN64(src + i) & + (AV_RN64(src + i) - 0x0100010001000101ULL)) & 0x8000800080008080ULL)) continue; FIND_FIRST_ZERO; @@ -63,8 +63,8 @@ int ff_h2645_extract_rbsp(const uint8_t *src, int length, } #else for (i = 0; i + 1 < length; i += 5) { -if (!((~AV_RN32A(src + i) & - (AV_RN32A(src + i) - 0x01000101U)) & +if (!((~AV_RN32(src + i) & + (AV_RN32(src + i) - 0x01000101U)) & 0x80008080U)) continue; FIND_FIRST_ZERO; -- 2.16.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/mpeg4videodec: Use more specific error codes
On Sat, Mar 10, 2018 at 03:33:33PM -0300, James Almer wrote: > On 3/10/2018 2:34 PM, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/mpeg4videodec.c | 100 > > +++-- > > 1 file changed, 51 insertions(+), 49 deletions(-) > > > > diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c > > index 19210d97fe..1357b357a8 100644 > > --- a/libavcodec/mpeg4videodec.c > > +++ b/libavcodec/mpeg4videodec.c > > @@ -448,7 +448,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext > > *ctx) > > > > /* is there enough space left for a video packet + header */ > > if (get_bits_count(&s->gb) > s->gb.size_in_bits - 20) > > -return -1; > > +return AVERROR_INVALIDDATA; > > > > for (len = 0; len < 32; len++) > > if (get_bits1(&s->gb)) > > @@ -456,7 +456,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext > > *ctx) > > > > if (len != ff_mpeg4_get_video_packet_prefix_length(s)) { > > av_log(s->avctx, AV_LOG_ERROR, "marker does not match f_code\n"); > > -return -1; > > +return AVERROR_INVALIDDATA; > > } > > > > if (ctx->shape != RECT_SHAPE) { > > @@ -468,7 +468,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext > > *ctx) > > if (mb_num >= s->mb_num || !mb_num) { > > av_log(s->avctx, AV_LOG_ERROR, > > "illegal mb_num in video packet (%d %d) \n", mb_num, > > s->mb_num); > > -return -1; > > +return AVERROR_INVALIDDATA; > > } > > > > s->mb_x = mb_num % s->mb_width; > > @@ -597,7 +597,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, > > int n, int *dir_ptr) > > > > if (code < 0 || code > 9 /* && s->nbit < 9 */) { > > av_log(s->avctx, AV_LOG_ERROR, "illegal dc vlc\n"); > > -return -1; > > +return AVERROR_INVALIDDATA; > > } > > > > if (code == 0) { > > @@ -620,7 +620,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, > > int n, int *dir_ptr) > > if (get_bits1(&s->gb) == 0) { /* marker */ > > if (s->avctx->err_recognition & > > (AV_EF_BITSTREAM|AV_EF_COMPLIANT)) { > > av_log(s->avctx, AV_LOG_ERROR, "dc marker bit > > missing\n"); > > -return -1; > > +return AVERROR_INVALIDDATA; > > } > > } > > } > > @@ -664,7 +664,7 @@ static int mpeg4_decode_partition_a(Mpeg4DecContext > > *ctx) > > if (cbpc < 0) { > > av_log(s->avctx, AV_LOG_ERROR, > > "mcbpc corrupted at %d %d\n", s->mb_x, > > s->mb_y); > > -return -1; > > +return cbpc; > > get_vlc2() seems to return -1 on error, so nothing really changes with > this. right, ill hardcode these > Same with every other similar call, the other calls should return proper error codes and we should forward these. Not forwarding an error because the current implementation of a function generates an error code that the caller can hardcode is bad design. We should not duplicate the implementation of what a function returns in the caller. The implementation could change and then this is wrong. Or am i missing a reason why hardcoding the error codes in the caller would be an advantage ? will send a new patch thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Does the universe only have a finite lifespan? No, its going to go on forever, its just that you wont like living in it. -- Hiranya Peiri signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/mpeg4videodec: Use more specific error codes
Forward error codes where possible. Signed-off-by: Michael Niedermayer --- libavcodec/mpeg4video.h| 4 +- libavcodec/mpeg4videodec.c | 100 +++-- 2 files changed, 53 insertions(+), 51 deletions(-) diff --git a/libavcodec/mpeg4video.h b/libavcodec/mpeg4video.h index 0ba502d50b..6672c6dd66 100644 --- a/libavcodec/mpeg4video.h +++ b/libavcodec/mpeg4video.h @@ -239,12 +239,12 @@ static inline int ff_mpeg4_pred_dc(MpegEncContext *s, int n, int level, if (level < 0) { av_log(s->avctx, AV_LOG_ERROR, "dc<0 at %dx%d\n", s->mb_x, s->mb_y); -return -1; +return AVERROR_INVALIDDATA; } if (level > 2048 + scale) { av_log(s->avctx, AV_LOG_ERROR, "dc overflow at %dx%d\n", s->mb_x, s->mb_y); -return -1; +return AVERROR_INVALIDDATA; } } if (level < 0) diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c index 19210d97fe..a1117dc272 100644 --- a/libavcodec/mpeg4videodec.c +++ b/libavcodec/mpeg4videodec.c @@ -448,7 +448,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext *ctx) /* is there enough space left for a video packet + header */ if (get_bits_count(&s->gb) > s->gb.size_in_bits - 20) -return -1; +return AVERROR_INVALIDDATA; for (len = 0; len < 32; len++) if (get_bits1(&s->gb)) @@ -456,7 +456,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext *ctx) if (len != ff_mpeg4_get_video_packet_prefix_length(s)) { av_log(s->avctx, AV_LOG_ERROR, "marker does not match f_code\n"); -return -1; +return AVERROR_INVALIDDATA; } if (ctx->shape != RECT_SHAPE) { @@ -468,7 +468,7 @@ int ff_mpeg4_decode_video_packet_header(Mpeg4DecContext *ctx) if (mb_num >= s->mb_num || !mb_num) { av_log(s->avctx, AV_LOG_ERROR, "illegal mb_num in video packet (%d %d) \n", mb_num, s->mb_num); -return -1; +return AVERROR_INVALIDDATA; } s->mb_x = mb_num % s->mb_width; @@ -597,7 +597,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, int n, int *dir_ptr) if (code < 0 || code > 9 /* && s->nbit < 9 */) { av_log(s->avctx, AV_LOG_ERROR, "illegal dc vlc\n"); -return -1; +return AVERROR_INVALIDDATA; } if (code == 0) { @@ -620,7 +620,7 @@ static inline int mpeg4_decode_dc(MpegEncContext *s, int n, int *dir_ptr) if (get_bits1(&s->gb) == 0) { /* marker */ if (s->avctx->err_recognition & (AV_EF_BITSTREAM|AV_EF_COMPLIANT)) { av_log(s->avctx, AV_LOG_ERROR, "dc marker bit missing\n"); -return -1; +return AVERROR_INVALIDDATA; } } } @@ -664,7 +664,7 @@ static int mpeg4_decode_partition_a(Mpeg4DecContext *ctx) if (cbpc < 0) { av_log(s->avctx, AV_LOG_ERROR, "mcbpc corrupted at %d %d\n", s->mb_x, s->mb_y); -return -1; +return AVERROR_INVALIDDATA; } } while (cbpc == 8); @@ -684,7 +684,7 @@ static int mpeg4_decode_partition_a(Mpeg4DecContext *ctx) if (dc < 0) { av_log(s->avctx, AV_LOG_ERROR, "DC corrupted at %d %d\n", s->mb_x, s->mb_y); -return -1; +return dc; } dir <<= 1; if (dc_pred_dir) @@ -736,7 +736,7 @@ try_again: if (cbpc < 0) { av_log(s->avctx, AV_LOG_ERROR, "mcbpc corrupted at %d %d\n", s->mb_x, s->mb_y); -return -1; +return AVERROR_INVALIDDATA; } if (cbpc == 20) goto try_again; @@ -774,11 +774,11 @@ try_again: if (!s->mcsel) { mx = ff_h263_decode_motion(s, pred_x, s->f_code); if (mx >= 0x) -return -1; +return AVERROR_INVALIDDATA; my = ff_h263_decode_motion(s, pred_y, s->f_code); if (my >= 0x) -return -1; +return AVERROR_INVALIDDATA; s->current_picture.mb_type[xy] = MB_TYPE_16x16 | MB_TYPE_L0; } else { @@ -805,11 +805,11 @@ try_again: int16_t *mot_val = ff_h263_pred_motion(s, i, 0, &pred_x, &pred_y);