Re: [FFmpeg-devel] [PATCH 3/3] avformat/rtpenc_mpegts: allow options for rtp muxer
Plan to push set tomorrow. On 2021-03-23 11:07, Gyan Doshi wrote: --- libavformat/rtpenc_mpegts.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libavformat/rtpenc_mpegts.c b/libavformat/rtpenc_mpegts.c index 22881461e4..f7ee5a4448 100644 --- a/libavformat/rtpenc_mpegts.c +++ b/libavformat/rtpenc_mpegts.c @@ -29,6 +29,7 @@ typedef struct MuxChain { AVFormatContext *rtp_ctx; AVPacket *pkt; AVDictionary* mpegts_muxer_options; +AVDictionary* rtp_muxer_options; } MuxChain; static int rtp_mpegts_write_close(AVFormatContext *s) @@ -59,6 +60,7 @@ static int rtp_mpegts_write_header(AVFormatContext *s) int i, ret = AVERROR(ENOMEM); AVStream *st; AVDictionary *mpegts_muxer_options = NULL; +AVDictionary *rtp_muxer_options = NULL; if (!mpegts_format || !rtp_format) return AVERROR(ENOSYS); @@ -108,7 +110,8 @@ static int rtp_mpegts_write_header(AVFormatContext *s) st->time_base.den = 9; st->codecpar->codec_id = AV_CODEC_ID_MPEG2TS; rtp_ctx->pb = s->pb; -if ((ret = avformat_write_header(rtp_ctx, NULL)) < 0) +av_dict_copy(&rtp_muxer_options, chain->rtp_muxer_options, 0); +if ((ret = avformat_write_header(rtp_ctx, &rtp_muxer_options)) < 0) goto fail; chain->rtp_ctx = rtp_ctx; @@ -121,6 +124,7 @@ fail: av_dict_free(&mpegts_muxer_options); avformat_free_context(mpegts_ctx); } +av_dict_free(&rtp_muxer_options); avformat_free_context(rtp_ctx); rtp_mpegts_write_close(s); return ret; @@ -167,6 +171,7 @@ static int rtp_mpegts_write_packet(AVFormatContext *s, AVPacket *pkt) #define E AV_OPT_FLAG_ENCODING_PARAM static const AVOption options[] = { { "mpegts_muxer_options", "set list of options for the MPEG-TS muxer", OFFSET(mpegts_muxer_options), AV_OPT_TYPE_DICT, {.str = NULL}, 0, 0, E }, +{ "rtp_muxer_options","set list of options for the RTP muxer", OFFSET(rtp_muxer_options),AV_OPT_TYPE_DICT, {.str = NULL}, 0, 0, E }, { NULL }, }; ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/rtpdec: Fix prft wallclock time.
Am Do., 25. März 2021 um 05:47 Uhr schrieb Alok Priyadarshi : > > Timestamp difference is available in media timebase (1/90K) where as > rtcp time is in the default microseconds timebase. This patch fixes > the calculated prft wallclock time by rescaling the timestamp delta > to the microseconds timebase. > --- > libavformat/rtpdec.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c > index b935dba1b8..21c1d01785 100644 > --- a/libavformat/rtpdec.c > +++ b/libavformat/rtpdec.c > @@ -585,14 +585,19 @@ void ff_rtp_parse_set_crypto(RTPDemuxContext *s, const > char *suite, > } > > static int rtp_set_prft(RTPDemuxContext *s, AVPacket *pkt, uint32_t > timestamp) { > +int64_t rtcp_time, delta_timestamp, delta_time; > + > AVProducerReferenceTime *prft = > (AVProducerReferenceTime *) av_packet_new_side_data( > pkt, AV_PKT_DATA_PRFT, sizeof(AVProducerReferenceTime)); > if (!prft) > return AVERROR(ENOMEM); > > -prft->wallclock = ff_parse_ntp_time(s->last_rtcp_ntp_time) - > NTP_OFFSET_US + > - timestamp - s->last_rtcp_timestamp; Wouldn't this patch get much more readable if you only replace this line? > +rtcp_time = ff_parse_ntp_time(s->last_rtcp_ntp_time) - NTP_OFFSET_US; > +delta_timestamp = (int64_t)timestamp - (int64_t)s->last_rtcp_timestamp; > +delta_time = av_rescale_q(delta_timestamp, s->st->time_base, > AV_TIME_BASE_Q); > + > +prft->wallclock = rtcp_time + delta_time; Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/utils: add helper functions to retrieve index entries from an AVStream
James Almer (12021-03-24): > I think it's clear by now that nothing i could say will convince you it's > better to not return a pointer to an internal array when there are safer > alternatives, and i already gave my reasons why, none of which satisfied > you, so i don't see the point in keeping this discussion going. I find this comment quite offensive. You did not manage to convince me because your arguments have not been up to the task. Do not try to push the fault on me, and I will refrain from accusing you of not taking my arguments into account. Coming to an agreement is a process, it requires both parts to refine their arguments progressively. This is a matter of choosing the least of several drawbacks. So let us compare the drawbacks and not muddle things further. For me: 1. having a dynamic allocation is way way worse than 2. having sizeof(AVIndexEntry) in the ABI, which is somewhat worse than 3. having a function with many arguments, which is a tiny bit worse than 4. having a "use this pointer immediately" constraint. We agree except on 3>4, so let us focus on that. Option (3) has these practical drawbacks: Many arguments involves more typing and the risk of messing with the order and getting invalid values. It also requires redesigning the API if we add fields and exporting them is useful. And it requires either the overhead of NULL checks or the caller declaring unneeded variables. Option (4) has the obvious practical drawback that misusing the API causes undefined behavior. The constraint of using a pointer immediately on risk of undefined behavior is actually a frequent one, in FFmpeg but also in C at large: gethosbtyname, localtime, etc. For me, that makes it approximately on par with the risk of messing the order of the many arguments. Which leaves more typing, NULL checks overhead or useless variables (still more typing). It is tiny, I have no trouble admitting, but it is tiny in favor of one solution. If you do not agree with these estimates, please explain exactly where. > If some other developer wants to chime in and comment which approach they > prefer, then that would be ideal. Indeed. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/utils: add helper functions to retrieve index entries from an AVStream
On 25 Mar 2021, at 12:55, Nicolas George wrote: > James Almer (12021-03-24): >> I think it's clear by now that nothing i could say will convince you it's >> better to not return a pointer to an internal array when there are safer >> alternatives, and i already gave my reasons why, none of which satisfied >> you, so i don't see the point in keeping this discussion going. > > I find this comment quite offensive. You did not manage to convince me > because your arguments have not been up to the task. Do not try to push > the fault on me, and I will refrain from accusing you of not taking my > arguments into account. Coming to an agreement is a process, it requires > both parts to refine their arguments progressively. > > This is a matter of choosing the least of several drawbacks. So let us > compare the drawbacks and not muddle things further. > > For me: > > 1. having a dynamic allocation is way way worse than > 2. having sizeof(AVIndexEntry) in the ABI, which is somewhat worse than > 3. having a function with many arguments, which is a tiny bit worse than > 4. having a "use this pointer immediately" constraint. > > We agree except on 3>4, so let us focus on that. > > Option (3) has these practical drawbacks: Many arguments involves more > typing and the risk of messing with the order and getting invalid > values. It also requires redesigning the API if we add fields and > exporting them is useful. And it requires either the overhead of NULL > checks or the caller declaring unneeded variables. > > Option (4) has the obvious practical drawback that misusing the API > causes undefined behavior. > > The constraint of using a pointer immediately on risk of undefined > behavior is actually a frequent one, in FFmpeg but also in C at large: > gethosbtyname, localtime, etc. > > For me, that makes it approximately on par with the risk of messing the > order of the many arguments. > > Which leaves more typing, NULL checks overhead or useless variables > (still more typing). > > It is tiny, I have no trouble admitting, but it is tiny in favor of one > solution. > > If you do not agree with these estimates, please explain exactly where. I disagree with your ordering too, for me 4. is clearly worse than 3. here, as the harm that can be done by mixing up arguments (in this case) is less than use of a possibly invalid pointer. And mixed up arguments would probably be noticed easier when testing than reads of possibly invalid memory. Even when documenting the constraint of immediately using the pointer, it could easily be overlooked/forgotten. It does not even has to be me forgetting the constraint, but could be someone else changing the code in the future, adding some API call before the pointer is used, unaware of the possible consequences and that could invalidate the pointer (if I understand James explanation correctly). This could go unnoticed easily. So IMO having a function with many arguments seems like a better and safer tradeoff in this case to me… > >> If some other developer wants to chime in and comment which approach they >> prefer, then that would be ideal. > > Indeed. > > Regards, > > -- > Nicolas George > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/utils: add helper functions to retrieve index entries from an AVStream
On 3/25/2021 8:55 AM, Nicolas George wrote: James Almer (12021-03-24): I think it's clear by now that nothing i could say will convince you it's better to not return a pointer to an internal array when there are safer alternatives, and i already gave my reasons why, none of which satisfied you, so i don't see the point in keeping this discussion going. I find this comment quite offensive. You did not manage to convince me because your arguments have not been up to the task. Do not try to push the fault on me, and I will refrain from accusing you of not taking my arguments into account. Coming to an agreement is a process, it requires both parts to refine their arguments progressively. It was not meant to be offensive. As i said above i have no other argument against your approach to this function than what i already said, hence why anything i might add will be more or less repeating myself. This is a matter of choosing the least of several drawbacks. So let us compare the drawbacks and not muddle things further. For me: 1. having a dynamic allocation is way way worse than 2. having sizeof(AVIndexEntry) in the ABI, which is somewhat worse than 3. having a function with many arguments, which is a tiny bit worse than 4. having a "use this pointer immediately" constraint. We agree except on 3>4, so let us focus on that. Option (3) has these practical drawbacks: Many arguments involves more typing and the risk of messing with the order and getting invalid values. It also requires redesigning the API if we add fields and exporting them is useful. And it requires either the overhead of NULL checks or the caller declaring unneeded variables. Same situation as av_add_index_entry(). And if you look at the signature of the (3) function in my last proposal, i kept the av_index_* namespace free precisely in case a new API in the future needs to be added for this reason, for both add() and get(). One that could work directly with the AVIndexEntry struct after the internal entries array was redesigned, perhaps using AVTreeNode, so returned pointers or entries are safer to handle. Option (4) has the obvious practical drawback that misusing the API causes undefined behavior. The constraint of using a pointer immediately on risk of undefined behavior is actually a frequent one, in FFmpeg but also in C at large: gethosbtyname, localtime, etc. For me, that makes it approximately on par with the risk of messing the order of the many arguments. Which leaves more typing, NULL checks overhead or useless variables (still more typing). It is tiny, I have no trouble admitting, but it is tiny in favor of one solution. If you do not agree with these estimates, please explain exactly where. I don't know if you remember how in this one imgutils patch i sent some time ago i was against functions with tons of output parameters, because i considered it ugly and typical of enterprise software API. That hasn't changed. But here, being the exact counterpart of an existing add() function put it above the other approach i dislike slightly more of returning an internal pointer and not being able to tell the user just what may invalidate it. If some other developer wants to chime in and comment which approach they prefer, then that would be ideal. Indeed. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/rtpdec: Fix prft wallclock time.
In effect the patch does replace that one line. But it also adds the steps to illustrate how the wallclock is calculated. Adding all the calculations in a single line will make it too long and hard to read. Note that delta_timestamp can be negative. It typically happens after rtcp SR is received and last_rtcp_ntp_time/last_rtcp_timestamp are refreshed. The packet timestamp can be less than last_rtcp_timestamp for a brief period of time. So it is necessary to explicitly cast both - timestamp and last_rtcp_timestamp - to int64 before calculating delta. This was another bug in the old code in addition to missing timebase scaling. On Thu, Mar 25, 2021 at 2:23 AM Carl Eugen Hoyos wrote: > Am Do., 25. März 2021 um 05:47 Uhr schrieb Alok Priyadarshi < > alo...@gmail.com>: > > > > Timestamp difference is available in media timebase (1/90K) where as > > rtcp time is in the default microseconds timebase. This patch fixes > > the calculated prft wallclock time by rescaling the timestamp delta > > to the microseconds timebase. > > --- > > libavformat/rtpdec.c | 9 +++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c > > index b935dba1b8..21c1d01785 100644 > > --- a/libavformat/rtpdec.c > > +++ b/libavformat/rtpdec.c > > @@ -585,14 +585,19 @@ void ff_rtp_parse_set_crypto(RTPDemuxContext *s, > const char *suite, > > } > > > > static int rtp_set_prft(RTPDemuxContext *s, AVPacket *pkt, uint32_t > timestamp) { > > +int64_t rtcp_time, delta_timestamp, delta_time; > > + > > AVProducerReferenceTime *prft = > > (AVProducerReferenceTime *) av_packet_new_side_data( > > pkt, AV_PKT_DATA_PRFT, sizeof(AVProducerReferenceTime)); > > if (!prft) > > return AVERROR(ENOMEM); > > > > -prft->wallclock = ff_parse_ntp_time(s->last_rtcp_ntp_time) - > NTP_OFFSET_US + > > > - timestamp - s->last_rtcp_timestamp; > > Wouldn't this patch get much more readable if you only replace this line? > > > +rtcp_time = ff_parse_ntp_time(s->last_rtcp_ntp_time) - > NTP_OFFSET_US; > > +delta_timestamp = (int64_t)timestamp - > (int64_t)s->last_rtcp_timestamp; > > +delta_time = av_rescale_q(delta_timestamp, s->st->time_base, > AV_TIME_BASE_Q); > > + > > +prft->wallclock = rtcp_time + delta_time; > > Carl Eugen > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Fw: Question about FFmpeg's threading architecture
Hi everyone, Kieran, thank you for your reply. I already had a look at that document. I need something more detailed about FFmpeg's threading architecture such as: 1. What threading model is used in FFmpeg (e.g., thread pool, pipeline, or peer)?2. Have someone explored the performance of the threading model?3. What are the threads and what task(s) are assigned to each thread? I appreciate it if someone can help me in understanding the FFmpeg's threading architecture. Regards, Alireza On Tuesday, March 23, 2021, 11:21:58 AM EDT, Kieran Kunhya wrote: Hi, On Tue, 23 Mar 2021 at 16:01, Alireza Heidar-Barghi < arhdr-at-yahoo@ffmpeg.org> wrote: > Hello, > I am wondering how I can obtain documentation on the detail of FFmpeg's > threading architecture? > Thank you in advance. > Regards, > Alireza > https://github.com/FFmpeg/FFmpeg/blob/master/doc/multithreading.txt Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 01/12] avcodec/vorbisenc: Remove always-false check
The PutBitContext is big enough: It has just been initialized to 8192B. Signed-off-by: Andreas Rheinhardt --- libavcodec/vorbisenc.c | 5 - 1 file changed, 5 deletions(-) diff --git a/libavcodec/vorbisenc.c b/libavcodec/vorbisenc.c index 18a679f2dc..f8a08f816f 100644 --- a/libavcodec/vorbisenc.c +++ b/libavcodec/vorbisenc.c @@ -1135,11 +1135,6 @@ static int vorbis_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, init_put_bits(&pb, avpkt->data, avpkt->size); -if (pb.size_in_bits - put_bits_count(&pb) < 1 + ilog(venc->nmodes - 1)) { -av_log(avctx, AV_LOG_ERROR, "output buffer is too small\n"); -return AVERROR(EINVAL); -} - put_bits(&pb, 1, 0); // magic bit put_bits(&pb, ilog(venc->nmodes - 1), 1); // Mode for current frame -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 02/12] avcodec/sonic: Remove outdated outcommented line
Compilation would fail if it were outcommented as it refers to a nonexistent PutBitContext. Signed-off-by: Andreas Rheinhardt --- libavcodec/sonic.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/libavcodec/sonic.c b/libavcodec/sonic.c index 32e94d24f6..8339799dd3 100644 --- a/libavcodec/sonic.c +++ b/libavcodec/sonic.c @@ -832,8 +832,6 @@ static int sonic_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, return ret; } -//av_log(avctx, AV_LOG_DEBUG, "used bytes: %d\n", (put_bits_count(&pb)+7)/8); - avpkt->size = ff_rac_terminate(&c, 0); *got_packet_ptr = 1; return 0; -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 03/12] avcodec/mlpenc: Avoid redundant temporary PutBitContext
We are already word-aligned here, so one can just as well flush the main PutBitContext. Signed-off-by: Andreas Rheinhardt --- libavcodec/mlpenc.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c index 9782cb8261..40db76f350 100644 --- a/libavcodec/mlpenc.c +++ b/libavcodec/mlpenc.c @@ -1067,7 +1067,7 @@ static uint8_t *write_substrs(MLPEncodeContext *ctx, uint8_t *buf, int buf_size, RestartHeader *rh = &ctx->restart_header [substr]; int substr_restart_frame = restart_frame; uint8_t parity, checksum; -PutBitContext pb, tmpb; +PutBitContext pb; int params_changed; ctx->cur_restart_header = rh; @@ -1117,9 +1117,9 @@ static uint8_t *write_substrs(MLPEncodeContext *ctx, uint8_t *buf, int buf_size, put_bits(&pb, 32, END_OF_STREAM); } -/* Data must be flushed for the checksum and parity to be correct. */ -tmpb = pb; -flush_put_bits(&tmpb); +/* Data must be flushed for the checksum and parity to be correct; + * notice that we already are word-aligned here. */ +flush_put_bits(&pb); parity = ff_mlp_calculate_parity(buf, put_bits_count(&pb) >> 3) ^ 0xa9; checksum = ff_mlp_checksum8 (buf, put_bits_count(&pb) >> 3); -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 04/12] avcodec/dvenc: Remove dead error message
The PutBits API checks the available space before every write, so this check for overread is dead. Signed-off-by: Andreas Rheinhardt --- libavcodec/dvenc.c | 5 - 1 file changed, 5 deletions(-) diff --git a/libavcodec/dvenc.c b/libavcodec/dvenc.c index 233e2b68c7..134315a834 100644 --- a/libavcodec/dvenc.c +++ b/libavcodec/dvenc.c @@ -980,11 +980,6 @@ static int dv_encode_video_segment(AVCodecContext *avctx, void *arg) int size = pbs[j].size_in_bits >> 3; flush_put_bits(&pbs[j]); pos = put_bits_count(&pbs[j]) >> 3; -if (pos > size) { -av_log(avctx, AV_LOG_ERROR, - "bitstream written beyond buffer size\n"); -return -1; -} memset(pbs[j].buf + pos, 0xff, size - pos); } -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 05/12] avcodec/vorbisenc, wmavoice: Use put_bits_left() where appropriate
Signed-off-by: Andreas Rheinhardt --- libavcodec/vorbisenc.c | 4 ++-- libavcodec/wmavoice.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libavcodec/vorbisenc.c b/libavcodec/vorbisenc.c index f8a08f816f..080017e2bf 100644 --- a/libavcodec/vorbisenc.c +++ b/libavcodec/vorbisenc.c @@ -155,7 +155,7 @@ static inline int put_codeword(PutBitContext *pb, vorbis_enc_codebook *cb, av_assert2(entry >= 0); av_assert2(entry < cb->nentries); av_assert2(cb->lens[entry]); -if (pb->size_in_bits - put_bits_count(pb) < cb->lens[entry]) +if (put_bits_left(pb) < cb->lens[entry]) return AVERROR(EINVAL); put_bits(pb, cb->lens[entry], cb->codewords[entry]); return 0; @@ -798,7 +798,7 @@ static int floor_encode(vorbis_enc_context *venc, vorbis_enc_floor *fc, int coded[MAX_FLOOR_VALUES]; // first 2 values are unused int i, counter; -if (pb->size_in_bits - put_bits_count(pb) < 1 + 2 * ilog(range - 1)) +if (put_bits_left(pb) < 1 + 2 * ilog(range - 1)) return AVERROR(EINVAL); put_bits(pb, 1, 1); // non zero put_bits(pb, ilog(range - 1), posts[0]); diff --git a/libavcodec/wmavoice.c b/libavcodec/wmavoice.c index fbdb865360..e76807faa5 100644 --- a/libavcodec/wmavoice.c +++ b/libavcodec/wmavoice.c @@ -1881,7 +1881,7 @@ static void copy_bits(PutBitContext *pb, rmn_bits = rmn_bytes = get_bits_left(gb); if (rmn_bits < nbits) return; -if (nbits > pb->size_in_bits - put_bits_count(pb)) +if (nbits > put_bits_left(pb)) return; rmn_bits &= 7; rmn_bytes >>= 3; if ((rmn_bits = FFMIN(rmn_bits, nbits)) > 0) -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 11/12] Avoid intermediate bitcount for number of bytes in PutBitContext
Signed-off-by: Andreas Rheinhardt --- libavcodec/aacenc.c| 4 ++-- libavcodec/alacenc.c | 2 +- libavcodec/asvenc.c| 2 +- libavcodec/cbs.c | 2 +- libavcodec/cfhdenc.c | 2 +- libavcodec/cljrenc.c | 2 +- libavcodec/dca.c | 2 +- libavcodec/dcaenc.c| 2 +- libavcodec/ffv1enc.c | 2 +- libavcodec/flacenc.c | 6 +++--- libavcodec/hevc_ps_enc.c | 2 +- libavcodec/lzwenc.c| 2 +- libavcodec/magicyuvenc.c | 6 ++ libavcodec/mjpegenc.c | 2 +- libavcodec/mjpegenc_common.c | 7 ++- libavcodec/mlpenc.c| 8 libavcodec/mpeg12enc.c | 2 +- libavcodec/mpeg4videoenc.c | 2 +- libavcodec/mpegaudioenc_template.c | 8 libavcodec/mpegvideo_enc.c | 11 ++- libavcodec/proresenc_anatoliy.c| 2 +- libavcodec/rpzaenc.c | 2 +- libavcodec/sbcenc.c| 2 +- libavcodec/sonic.c | 2 +- libavcodec/svq1enc.c | 2 +- libavcodec/tests/cabac.c | 2 +- libavcodec/ttaenc.c| 2 +- libavcodec/vorbisenc.c | 8 libavcodec/wavpackenc.c| 4 ++-- libavcodec/wmaenc.c| 2 +- libavcodec/xsubenc.c | 4 ++-- libavfilter/vf_signature.c | 2 +- libavformat/latmenc.c | 2 +- libavformat/movenc.c | 2 +- 34 files changed, 55 insertions(+), 59 deletions(-) diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c index 070a2e706a..6dc68eb9b0 100644 --- a/libavcodec/aacenc.c +++ b/libavcodec/aacenc.c @@ -115,7 +115,7 @@ static int put_audio_specific_config(AVCodecContext *avctx) put_bits(&pb, 5, AOT_SBR); put_bits(&pb, 1, 0); flush_put_bits(&pb); -avctx->extradata_size = put_bits_count(&pb) >> 3; +avctx->extradata_size = put_bytes_output(&pb); return 0; } @@ -881,6 +881,7 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, flush_put_bits(&s->pb); s->last_frame_pb_count = put_bits_count(&s->pb); +avpkt->size= put_bytes_output(&s->pb); s->lambda_sum += s->lambda; s->lambda_count++; @@ -888,7 +889,6 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, ff_af_queue_remove(&s->afq, avctx->frame_size, &avpkt->pts, &avpkt->duration); -avpkt->size = put_bits_count(&s->pb) >> 3; *got_packet_ptr = 1; return 0; } diff --git a/libavcodec/alacenc.c b/libavcodec/alacenc.c index 9d135d1350..ecdd46cac3 100644 --- a/libavcodec/alacenc.c +++ b/libavcodec/alacenc.c @@ -485,7 +485,7 @@ static int write_frame(AlacEncodeContext *s, AVPacket *avpkt, put_bits(pb, 3, TYPE_END); flush_put_bits(pb); -return put_bits_count(pb) >> 3; +return put_bytes_output(pb); } static av_always_inline int get_max_frame_size(int frame_size, int ch, int bps) diff --git a/libavcodec/asvenc.c b/libavcodec/asvenc.c index 2d8c310521..c33a3b4681 100644 --- a/libavcodec/asvenc.c +++ b/libavcodec/asvenc.c @@ -290,7 +290,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, else flush_put_bits_le(&a->pb); AV_WN32(put_bits_ptr(&a->pb), 0); -size = (put_bits_count(&a->pb) + 31) / 32; +size = (put_bytes_output(&a->pb) + 3) / 4; if (avctx->codec_id == AV_CODEC_ID_ASV1) { a->bbdsp.bswap_buf((uint32_t *) pkt->data, diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index c7f69845fb..bbfafb6530 100644 --- a/libavcodec/cbs.c +++ b/libavcodec/cbs.c @@ -346,7 +346,7 @@ static int cbs_write_unit_data(CodedBitstreamContext *ctx, flush_put_bits(&pbc); -ret = ff_cbs_alloc_unit_data(unit, put_bits_count(&pbc) / 8); +ret = ff_cbs_alloc_unit_data(unit, put_bytes_output(&pbc)); if (ret < 0) return ret; diff --git a/libavcodec/cfhdenc.c b/libavcodec/cfhdenc.c index 42bbf99c96..0369164fab 100644 --- a/libavcodec/cfhdenc.c +++ b/libavcodec/cfhdenc.c @@ -766,7 +766,7 @@ static int cfhd_encode_frame(AVCodecContext *avctx, AVPacket *pkt, put_bits(pb, cb[512].size, cb[512].bits); flush_put_bits(pb); -bytestream2_skip_p(pby, put_bits_count(pb) >> 3); +bytestream2_skip_p(pby, put_bytes_output(pb)); padd = (4 - (bytestream2_tell_p(pby) & 3)) & 3; while (padd--) bytestream2_put_byte(pby, 0); diff --git a/libavcodec/cljrenc.c b/libavcodec/cljrenc.c index a3718259d1..fe58f50f09 100644 --- a/libavcodec/cljrenc.c +++ b/libavcodec/cljrenc.c @@ -89,7 +89,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, flush_put_bits(&pb); -pkt->size = put_bits_count(&pb) / 8; +pkt->size = put_bytes_output(&pb); pkt->flags |= AV_PK
[FFmpeg-devel] [PATCH 06/12] avcodec/put_bits: Add functions for amount of bytes written/left
Often a caller doesn't want the amount of bits written via a PutBitContext, but the amount of bytes. This in particular happens after one has flushed the PutBitContext (e.g. at the end of encoding, when one wants to know the actual packet size). The current way of doing this is with put_bits_count(pb)/8 (or (put_bits_count(pb) + 7)/8). Yet this has some issues: It contains implicit multiplications and divisions by 8 with a cast in between; it obscurs the intent; and it restricts the size of the buffer to (currently) INT_MAX/8 (or to 1/8 of the maximum of whatever put_bits_count() returns), although said restriction is not really necessary for users that don't need a bitcount. Corresponding functions for the amount of bytes left have also been addded. Signed-off-by: Andreas Rheinhardt --- libavcodec/put_bits.h | 30 ++ 1 file changed, 30 insertions(+) diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h index cd66a82a44..e8bc86a82c 100644 --- a/libavcodec/put_bits.h +++ b/libavcodec/put_bits.h @@ -85,6 +85,26 @@ static inline int put_bits_count(PutBitContext *s) return (s->buf_ptr - s->buf) * 8 + BUF_BITS - s->bit_left; } +/** + * @return the number of bytes output so far; may only be called + * when the PutBitContext is freshly initialized or flushed. + */ +static inline int put_bytes_output(const PutBitContext *s) +{ +av_assert2(s->bit_left == BUF_BITS); +return s->buf_ptr - s->buf; +} + +/** + * @param round_up When set, the number of bits written so far will be + * rounded up to the next byte. + * @return the number of bytes output so far. + */ +static inline int put_bytes_count(const PutBitContext *s, int round_up) +{ +return s->buf_ptr - s->buf + ((BUF_BITS - s->bit_left + (round_up ? 7 : 0)) >> 3); +} + /** * Rebase the bit writer onto a reallocated buffer. * @@ -111,6 +131,16 @@ static inline int put_bits_left(PutBitContext* s) return (s->buf_end - s->buf_ptr) * 8 - BUF_BITS + s->bit_left; } +/** + * @param round_up When set, the number of bits written will be + * rounded up to the next byte. + * @return the number of bytes left. + */ +static inline int put_bytes_left(const PutBitContext *s, int round_up) +{ +return s->buf_end - s->buf_ptr - ((BUF_BITS - s->bit_left + (round_up ? 7 : 0)) >> 3); +} + /** * Pad the end of the output stream with zeros. */ -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 12/12] avcodec/put_bits: Don't set size_in_bits, fix overflow
A PutBitContext has a field called size_in_bits which is set to the context's bitsize init_put_bits(); but it isn't used at all (the PutBits API uses pointers directly and not bit indexes), so remove it (due to ABI concerns the actual element is only removed at the next bump). Furthermore, the multiplication inherent in setting this field can lead to undefined integer overflows. This is particularly true for FFV1, which uses a very big worst-case buffer (37*4*width*height; even ordinary 1080p triggers an overflow). Ticket #8350 is about this overflow which this commit fixes. This means that the effective range of the PutBits API is no longer restricted by the /8 as long as one isn't using put_bits_(count|left). Signed-off-by: Andreas Rheinhardt --- 1. One can use the full range of int (or of whatever type we choose) and check for whether enough bits are left with a function like this: static inline int put_bits_is_left(const PutBitContext *s, int n) { s->buf_end - s->buf_ptr >= (BUF_BITS - s->bit_left + n + 7) / 8; } (This presumes that n is not nearly the maximum of its type.) 2. I see three more ways in which the PutBits API can be improved: a) Don't use av_log/asserts in case of overreads; instead handle it via a context field for overread similar to the bytestream API. b) Integrate the PutBits API and the bytestream API: When one knows that the current position in the bitstream is byte-aligned and the PutBits context is flushed, one should be able to use the PutBits API with an API similar to the one for PutByteContexts. ff_mjpeg_encode_picture_header() is an example where this would be beneficial. c) If the output buffer is suitably padded, one could allow slight overwrites. In this case the check s->buf_end - s->buf_ptr >= sizeof(BitBuf) could be replaced by s->buf_ptr < s->buf_end (one could also add a new pointer to the context for the effective end (s->buf_end - FFMIN(buffer_size, sizeof(BitBuf))) and compare with that instead, but this seems to be a bit too involved). What do others think about this? libavcodec/put_bits.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h index e8bc86a82c..15c2650724 100644 --- a/libavcodec/put_bits.h +++ b/libavcodec/put_bits.h @@ -52,7 +52,9 @@ typedef struct PutBitContext { BitBuf bit_buf; int bit_left; uint8_t *buf, *buf_ptr, *buf_end; +#if LIBAVCODEC_VERSION_MAJOR < 59 int size_in_bits; +#endif } PutBitContext; /** @@ -69,7 +71,6 @@ static inline void init_put_bits(PutBitContext *s, uint8_t *buffer, buffer = NULL; } -s->size_in_bits = 8 * buffer_size; s->buf = buffer; s->buf_end = s->buf + buffer_size; s->buf_ptr = s->buf; @@ -120,7 +121,6 @@ static inline void rebase_put_bits(PutBitContext *s, uint8_t *buffer, s->buf_end = buffer + buffer_size; s->buf_ptr = buffer + (s->buf_ptr - s->buf); s->buf = buffer; -s->size_in_bits = 8 * buffer_size; } /** @@ -414,7 +414,6 @@ static inline void set_put_bits_buffer_size(PutBitContext *s, int size) { av_assert0(size <= INT_MAX/8 - BUF_BITS); s->buf_end = s->buf + size; -s->size_in_bits = 8*size; } /** -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 07/12] avcodec: Remove cumbersome way of checking for amount of bytes left
Several encoders used code like the following to check for the amount of bytes left in a PutBitContext: pb->buf_end - pb->buf - (put_bits_count(pb) >> 3) Besides the fact that using the pointers directly might pose a maintainence burden in the future this also leads to suboptimal code: The above code reads all three pointers (buf, buf_ptr and buf_end), but touching buf is unnecessary and switching to put_bytes_left() automatically fixes this. Signed-off-by: Andreas Rheinhardt --- libavcodec/asvenc.c | 2 +- libavcodec/ffv1enc_template.c | 2 +- libavcodec/huffyuvenc.c | 9 - libavcodec/ljpegenc.c | 4 ++-- libavcodec/mpegvideo_enc.c| 13 ++--- libavcodec/svq1enc.c | 3 +-- libavcodec/xsubenc.c | 2 +- 7 files changed, 16 insertions(+), 19 deletions(-) diff --git a/libavcodec/asvenc.c b/libavcodec/asvenc.c index 718ffa1836..2d8c310521 100644 --- a/libavcodec/asvenc.c +++ b/libavcodec/asvenc.c @@ -167,7 +167,7 @@ static inline int encode_mb(ASV1Context *a, int16_t block[6][64]) { int i; -av_assert0(a->pb.buf_end - a->pb.buf - (put_bits_count(&a->pb) >> 3) >= MAX_MB_SIZE); +av_assert0(put_bytes_left(&a->pb, 0) >= MAX_MB_SIZE); if (a->avctx->codec_id == AV_CODEC_ID_ASV1) { for (i = 0; i < 6; i++) diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c index bc0add5ed7..8a4a387923 100644 --- a/libavcodec/ffv1enc_template.c +++ b/libavcodec/ffv1enc_template.c @@ -37,7 +37,7 @@ static av_always_inline int RENAME(encode_line)(FFV1Context *s, int w, return AVERROR_INVALIDDATA; } } else { -if (s->pb.buf_end - s->pb.buf - (put_bits_count(&s->pb) >> 3) < w * 4) { +if (put_bytes_left(&s->pb, 0) < w * 4) { av_log(s->avctx, AV_LOG_ERROR, "encoded frame too large\n"); return AVERROR_INVALIDDATA; } diff --git a/libavcodec/huffyuvenc.c b/libavcodec/huffyuvenc.c index 28a5534535..2882433db5 100644 --- a/libavcodec/huffyuvenc.c +++ b/libavcodec/huffyuvenc.c @@ -443,7 +443,7 @@ static int encode_422_bitstream(HYuvContext *s, int offset, int count) const uint8_t *u = s->temp[1] + offset / 2; const uint8_t *v = s->temp[2] + offset / 2; -if (s->pb.buf_end - s->pb.buf - (put_bits_count(&s->pb) >> 3) < 2 * 4 * count) { +if (put_bytes_left(&s->pb, 0) < 2 * 4 * count) { av_log(s->avctx, AV_LOG_ERROR, "encoded frame too large\n"); return -1; } @@ -495,7 +495,7 @@ static int encode_plane_bitstream(HYuvContext *s, int width, int plane) { int i, count = width/2; -if (s->pb.buf_end - s->pb.buf - (put_bits_count(&s->pb) >> 3) < count * s->bps / 2) { +if (put_bytes_left(&s->pb, 0) < count * s->bps / 2) { av_log(s->avctx, AV_LOG_ERROR, "encoded frame too large\n"); return -1; } @@ -657,7 +657,7 @@ static int encode_gray_bitstream(HYuvContext *s, int count) { int i; -if (s->pb.buf_end - s->pb.buf - (put_bits_count(&s->pb) >> 3) < 4 * count) { +if (put_bytes_left(&s->pb, 0) < 4 * count) { av_log(s->avctx, AV_LOG_ERROR, "encoded frame too large\n"); return -1; } @@ -702,8 +702,7 @@ static inline int encode_bgra_bitstream(HYuvContext *s, int count, int planes) { int i; -if (s->pb.buf_end - s->pb.buf - (put_bits_count(&s->pb) >> 3) < -4 * planes * count) { +if (put_bytes_left(&s->pb, 0) < 4 * planes * count) { av_log(s->avctx, AV_LOG_ERROR, "encoded frame too large\n"); return -1; } diff --git a/libavcodec/ljpegenc.c b/libavcodec/ljpegenc.c index 3c68c08a3c..056b80b4b5 100644 --- a/libavcodec/ljpegenc.c +++ b/libavcodec/ljpegenc.c @@ -86,7 +86,7 @@ FF_ENABLE_DEPRECATION_WARNINGS const int modified_predictor = y ? s->pred : 1; uint8_t *ptr = frame->data[0] + (linesize * y); -if (pb->buf_end - pb->buf - (put_bits_count(pb) >> 3) < width * 4 * 4) { +if (put_bytes_left(pb, 0) < width * 4 * 4) { av_log(avctx, AV_LOG_ERROR, "encoded frame too large\n"); return -1; } @@ -211,7 +211,7 @@ FF_ENABLE_DEPRECATION_WARNINGS #endif for (mb_y = 0; mb_y < mb_height; mb_y++) { -if (pb->buf_end - pb->buf - (put_bits_count(pb) >> 3) < +if (put_bytes_left(pb, 0) < mb_width * 4 * 3 * s->hsample[0] * s->vsample[0]) { av_log(avctx, AV_LOG_ERROR, "encoded frame too large\n"); return -1; diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c index 7eecfce27c..8731b0ad31 100644 --- a/libavcodec/mpegvideo_enc.c +++ b/libavcodec/mpegvideo_enc.c @@ -1993,8 +1993,7 @@ FF_ENABLE_DEPRECATION_WARNINGS stuffing_count = ff_vbv_update(s, s->frame_bits); s->stuffing_bits = 8*stuffing_count; if (stuffing_count) { -if (s->pb.buf_end - s->pb.buf - (put_bits_count(&s->pb) >> 3) < -stuffing_count + 50) { +
[FFmpeg-devel] [PATCH 08/12] avcodec/utvideoenc: Don't use bitcounts when byte-aligned
Despite write_huff_codes() receiving an ordinary buffer (not a PutBitContext), it returned the amount of data written in bits, not in bytes. This has been changed: There is now no intermediate bitcount any more. Signed-off-by: Andreas Rheinhardt --- libavcodec/utvideoenc.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libavcodec/utvideoenc.c b/libavcodec/utvideoenc.c index 5c87eb50ac..32c204a898 100644 --- a/libavcodec/utvideoenc.c +++ b/libavcodec/utvideoenc.c @@ -398,13 +398,11 @@ static int write_huff_codes(uint8_t *src, uint8_t *dst, int dst_size, if (count) put_bits(&pb, 32 - count, 0); -/* Get the amount of bits written */ -count = put_bits_count(&pb); - /* Flush the rest with zeroes */ flush_put_bits(&pb); -return count; +/* Return the amount of bytes written */ +return put_bytes_output(&pb); } static int encode_plane(AVCodecContext *avctx, uint8_t *src, @@ -512,11 +510,11 @@ static int encode_plane(AVCodecContext *avctx, uint8_t *src, /* * Write the huffman codes to a buffer, - * get the offset in bits and convert to bytes. + * get the offset in bytes. */ offset += write_huff_codes(dst + sstart * width, c->slice_bits, width * height + 4, width, - send - sstart, he) >> 3; + send - sstart, he); slice_len = offset - slice_len; -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 09/12] avcodec/proresenc_kostya: Factor flushing PutBitContext out
The function to write an ordinary (luma or chroma) plane as well as the function for writing an alpha plane have some similarities: They record the initial bitposition (despite said position always being byte-aligned), flush the PutBitContext themselves and return the amount of bytes they wrote. This commit factors this out; it also replaces bitpositions by bytepositions and it avoids recording the initial byteposition because said information is already available from the position at the end of the last plane. Signed-off-by: Andreas Rheinhardt --- libavcodec/proresenc_kostya.c | 36 +-- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c index 0e70163bcc..d8edd12f34 100644 --- a/libavcodec/proresenc_kostya.c +++ b/libavcodec/proresenc_kostya.c @@ -464,23 +464,17 @@ static void encode_acs(PutBitContext *pb, int16_t *blocks, } } -static int encode_slice_plane(ProresContext *ctx, PutBitContext *pb, +static void encode_slice_plane(ProresContext *ctx, PutBitContext *pb, const uint16_t *src, ptrdiff_t linesize, int mbs_per_slice, int16_t *blocks, int blocks_per_mb, int plane_size_factor, const int16_t *qmat) { -int blocks_per_slice, saved_pos; - -saved_pos = put_bits_count(pb); -blocks_per_slice = mbs_per_slice * blocks_per_mb; +int blocks_per_slice = mbs_per_slice * blocks_per_mb; encode_dcs(pb, blocks, blocks_per_slice, qmat[0]); encode_acs(pb, blocks, blocks_per_slice, plane_size_factor, ctx->scantable, qmat); -flush_put_bits(pb); - -return (put_bits_count(pb) - saved_pos) >> 3; } static void put_alpha_diff(PutBitContext *pb, int cur, int prev, int abits) @@ -516,14 +510,13 @@ static void put_alpha_run(PutBitContext *pb, int run) } // todo alpha quantisation for high quants -static int encode_alpha_plane(ProresContext *ctx, PutBitContext *pb, +static void encode_alpha_plane(ProresContext *ctx, PutBitContext *pb, int mbs_per_slice, uint16_t *blocks, int quant) { const int abits = ctx->alpha_bits; const int mask = (1 << abits) - 1; const int num_coeffs = mbs_per_slice * 256; -int saved_pos = put_bits_count(pb); int prev = mask, cur; int idx = 0; int run = 0; @@ -544,8 +537,6 @@ static int encode_alpha_plane(ProresContext *ctx, PutBitContext *pb, } while (idx < num_coeffs); if (run) put_alpha_run(pb, run); -flush_put_bits(pb); -return (put_bits_count(pb) - saved_pos) >> 3; } static int encode_slice(AVCodecContext *avctx, const AVFrame *pic, @@ -611,24 +602,23 @@ static int encode_slice(AVCodecContext *avctx, const AVFrame *pic, ctx->blocks[0], ctx->emu_buf, mbs_per_slice, num_cblocks, is_chroma); if (!is_chroma) {/* luma quant */ -sizes[i] = encode_slice_plane(ctx, pb, src, linesize, - mbs_per_slice, ctx->blocks[0], - num_cblocks, plane_factor, - qmat); +encode_slice_plane(ctx, pb, src, linesize, + mbs_per_slice, ctx->blocks[0], + num_cblocks, plane_factor, qmat); } else { /* chroma plane */ -sizes[i] = encode_slice_plane(ctx, pb, src, linesize, - mbs_per_slice, ctx->blocks[0], - num_cblocks, plane_factor, - qmat_chroma); +encode_slice_plane(ctx, pb, src, linesize, + mbs_per_slice, ctx->blocks[0], + num_cblocks, plane_factor, qmat_chroma); } } else { get_alpha_data(ctx, src, linesize, xp, yp, pwidth, avctx->height / ctx->pictures_per_frame, ctx->blocks[0], mbs_per_slice, ctx->alpha_bits); -sizes[i] = encode_alpha_plane(ctx, pb, mbs_per_slice, - ctx->blocks[0], quant); +encode_alpha_plane(ctx, pb, mbs_per_slice, ctx->blocks[0], quant); } -total_size += sizes[i]; +flush_put_bits(pb); +sizes[i] = put_bytes_output(pb) - total_size; +total_size = put_bytes_output(pb); if (put_bits_left(pb) < 0) { av_log(avctx, AV_LOG_ERROR, "Underestimated required buffer size.\n"); -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffm
[FFmpeg-devel] [PATCH 10/12] avcodec/dvenc: Avoid using PutBitContext fields directly
Also avoid using bitcounts in case one is actually byte-aligned. Signed-off-by: Andreas Rheinhardt --- libavcodec/dvenc.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libavcodec/dvenc.c b/libavcodec/dvenc.c index 134315a834..e0679c538c 100644 --- a/libavcodec/dvenc.c +++ b/libavcodec/dvenc.c @@ -976,11 +976,8 @@ static int dv_encode_video_segment(AVCodecContext *avctx, void *arg) } for (j = 0; j < 5 * s->sys->bpm; j++) { -int pos; -int size = pbs[j].size_in_bits >> 3; flush_put_bits(&pbs[j]); -pos = put_bits_count(&pbs[j]) >> 3; -memset(pbs[j].buf + pos, 0xff, size - pos); +memset(put_bits_ptr(&pbs[j]), 0xff, put_bytes_left(&pbs[j], 0)); } if (DV_PROFILE_IS_HD(s->sys)) -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCHv2] avformat/mov: Properly forward error codes from av_seek
This is important, for example, for connection timed out events, when used over a network, returning AVERROR(ETIMEDOUT). Signed-off-by: Derek Buitenhuis --- Sent the wrong version the first time, woops. --- libavformat/mov.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavformat/mov.c b/libavformat/mov.c index aef5517c2c..f12f174924 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7947,6 +7947,8 @@ static int mov_read_packet(AVFormatContext *s, AVPacket *pkt) sc->ffindex, sample->pos); if (should_retry(sc->pb, ret64)) { mov_current_sample_dec(sc); +} else if (ret64 < 0) { +return (int)ret64; } return AVERROR_INVALIDDATA; } -- 2.31.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/mov: Properly forward error codes from av_seek
This is important, for example, for connection timed out events, when used ovr a network, returning AVERROR(ETIMEDOUT). Signed-off-by: Derek Buitenhuis --- libavformat/mov.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index aef5517c2c..bd78e68c16 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7947,8 +7947,10 @@ static int mov_read_packet(AVFormatContext *s, AVPacket *pkt) sc->ffindex, sample->pos); if (should_retry(sc->pb, ret64)) { mov_current_sample_dec(sc); -} -return AVERROR_INVALIDDATA; +} else if (ret64 < 0) +return (int)ret64; +else +return AVERROR_INVALIDDATA; } if (st->discard == AVDISCARD_NONKEY && !(sample->flags & AVINDEX_KEYFRAME)) { -- 2.31.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/sga: Check for array end in lzss_decompress()
On Thu, Mar 18, 2021 at 12:17:27AM +0100, Michael Niedermayer wrote: > Fixes: out of array access > Fixes: > 31640/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SGA_fuzzer-5630883286614016 > Fixes: > 31619/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SGA_fuzzer-5176667708456960 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/sga.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/3] avformat/mov: Check sample size for overflow in mov_parse_stsd_audio()
On Thu, Mar 18, 2021 at 12:17:28AM +0100, Michael Niedermayer wrote: > Fixes: signed integer overflow: 2 * 1914708000 cannot be represented in type > 'int' > Fixes: > 31639/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-6303428239294464 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavformat/mov.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In a rich man's house there is no place to spit but his face. -- Diogenes of Sinope signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/4] tools/target_dec_fuzzer: Adjust VP4 threshold
On Thu, Mar 18, 2021 at 09:49:56PM +0100, Michael Niedermayer wrote: > Fixes: Timeout (>10sec -> <100ms) > Fixes: > 31515/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP4_fuzzer-5247114134290432 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > tools/target_dec_fuzzer.c | 1 + > 1 file changed, 1 insertion(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Breaking DRM is a little like attempting to break through a door even though the window is wide open and the only thing in the house is a bunch of things you dont want and which you would get tomorrow for free anyway signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/4] avformat/avidec: Check for dv streams before using priv_data in parse ##dc/##wb
On Fri, Mar 19, 2021 at 01:19:28AM +0100, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Fri, Mar 19, 2021 at 12:20:23AM +0100, Andreas Rheinhardt wrote: > >> Michael Niedermayer: > >>> Fixes: null pointer dereference > >>> Fixes: > >>> 31588/clusterfuzz-testcase-minimized-ffmpeg_dem_AVI_fuzzer-6165716135968768 > >>> > >>> Found-by: continuous fuzzing process > >>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > >>> Signed-off-by: Michael Niedermayer > >>> --- > >>> libavformat/avidec.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/libavformat/avidec.c b/libavformat/avidec.c > >>> index fa0599501a..48370fe5ce 100644 > >>> --- a/libavformat/avidec.c > >>> +++ b/libavformat/avidec.c > >>> @@ -1288,7 +1288,7 @@ start_sync: > >>> AVStream *st1 = s->streams[1]; > >>> AVIStream *ast1 = st1->priv_data; > >>> // workaround for broken small-file-bug402.avi > >>> -if ( d[2] == 'w' && d[3] == 'b' > >>> +if (ast1 && d[2] == 'w' && d[3] == 'b' > >>> && n == 0 > >>> && st ->codecpar->codec_type == AVMEDIA_TYPE_VIDEO > >>> && st1->codecpar->codec_type == AVMEDIA_TYPE_AUDIO > >>> > >> How is this possible? After all, dv streams also have an AVIStream as > > > > The DV demuxer creates streams in dv_extract_audio_info() without a > > AVIStream > > > > That explains it. Thanks. Patch is fine by me, will apply thx > but I haven't looked at > it in detail. But neither dv nor avi set the AVFMTCTX_NOHEADER flag, so > adding streams later is an API violation. > > > > >> priv_data; and only the very first stream can ever be a dv stream due to > >> the check in line 605. > > > > I assume they are created after that check > > > > > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". -- 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 https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: Avoid loosing cluster array on failure
On Thu, Mar 18, 2021 at 09:49:57PM +0100, Michael Niedermayer wrote: > Fixes: crash > Fixes: check_pkt.mp4 > > Found-by: Rafael Dutra > Signed-off-by: Michael Niedermayer > --- > libavformat/movenc.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you think the mosad wants you dead since a long time then you are either wrong or dead since a long time. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/aacpsy: Check model_priv_data before dereferencing in psy_3gpp_end()
On Fri, Mar 19, 2021 at 04:26:51PM +0100, Michael Niedermayer wrote: > Fixes: null pointer dereference > Fixes: av_freep.mp4 > > Found-by: Rafael Dutra > Tested-by: Rafael Dutra > Signed-off-by: Michael Niedermayer > --- > libavcodec/aacpsy.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/6] avcodec/h264_slice: Check sps in h264_slice_header_init()
On Mon, Mar 22, 2021 at 09:58:29PM +0100, Michael Niedermayer wrote: > Fixes: null pointer dereference > Fixes: h264_slice_header_init.mp4 > > Found-by: Rafael Dutra > Tested-by: Rafael Dutra > Signed-off-by: Michael Niedermayer > --- > libavcodec/h264_slice.c | 5 + > 1 file changed, 5 insertions(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The day soldiers stop bringing you their problems is the day you have stopped leading them. They have either lost confidence that you can help or concluded you do not care. Either case is a failure of leadership. - Colin Powell signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 6/6] tools/target_dec_fuzzer: Adjust threshold for flac
On Mon, Mar 22, 2021 at 09:58:33PM +0100, Michael Niedermayer wrote: > Fixes: Timeout > Fixes: > 31464/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FLAC_fuzzer-4843965653319680 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > tools/target_dec_fuzzer.c | 1 + > 1 file changed, 1 insertion(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you fake or manipulate statistics in a paper in physics you will never get a job again. If you fake or manipulate statistics in a paper in medicin you will get a job for life at the pharma industry. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/sga: Check for array end in lzss_decompress()
please remove excessive whitespaces On Thu, Mar 25, 2021 at 6:09 PM Michael Niedermayer wrote: > On Thu, Mar 18, 2021 at 12:17:27AM +0100, Michael Niedermayer wrote: > > Fixes: out of array access > > Fixes: > 31640/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SGA_fuzzer-5630883286614016 > > Fixes: > 31619/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SGA_fuzzer-5176667708456960 > > > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/sga.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > will apply > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Its not that you shouldnt use gotos but rather that you should write > readable code and code with gotos often but not always is less readable > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/utils: add helper functions to retrieve index entries from an AVStream
On Thu, Mar 25, 2021 at 02:21:43PM +0100, Marvin Scholz wrote: > On 25 Mar 2021, at 12:55, Nicolas George wrote: > > James Almer (12021-03-24): > >> I think it's clear by now that nothing i could say will convince you it's > >> better to not return a pointer to an internal array when there are safer > >> alternatives, and i already gave my reasons why, none of which satisfied > >> you, so i don't see the point in keeping this discussion going. > > > > I find this comment quite offensive. You did not manage to convince me > > because your arguments have not been up to the task. Do not try to push > > the fault on me, and I will refrain from accusing you of not taking my > > arguments into account. Coming to an agreement is a process, it requires > > both parts to refine their arguments progressively. > > > > This is a matter of choosing the least of several drawbacks. So let us > > compare the drawbacks and not muddle things further. > > > > For me: > > > > 1. having a dynamic allocation is way way worse than > > 2. having sizeof(AVIndexEntry) in the ABI, which is somewhat worse than > > 3. having a function with many arguments, which is a tiny bit worse than > > 4. having a "use this pointer immediately" constraint. > > > > We agree except on 3>4, so let us focus on that. > > > > Option (3) has these practical drawbacks: Many arguments involves more > > typing and the risk of messing with the order and getting invalid > > values. It also requires redesigning the API if we add fields and > > exporting them is useful. And it requires either the overhead of NULL > > checks or the caller declaring unneeded variables. > > > > Option (4) has the obvious practical drawback that misusing the API > > causes undefined behavior. > > > > The constraint of using a pointer immediately on risk of undefined > > behavior is actually a frequent one, in FFmpeg but also in C at large: > > gethosbtyname, localtime, etc. > > > > For me, that makes it approximately on par with the risk of messing the > > order of the many arguments. > > > > Which leaves more typing, NULL checks overhead or useless variables > > (still more typing). > > > > It is tiny, I have no trouble admitting, but it is tiny in favor of one > > solution. > > > > If you do not agree with these estimates, please explain exactly where. > > I disagree with your ordering too, > for me 4. is clearly worse than 3. here, as the harm that can be > done by mixing up arguments (in this case) is less than use of a > possibly invalid pointer. If you mess the order of the arguments in an unlucky way or forgot to initialize one, that's also undefined behavior--though you may get warnings. > And mixed up arguments would probably be noticed easier when testing > than reads of possibly invalid memory. Every operation is potentially invalid unless you make sure that you keep the constraints given by the API. That's the end of the story. Returning to the previous streams example, you also have to make sure, you do not call avformat_free_context() somewhere before accessing AVStream, because that would invalidate that. You can do it easily without getting any warnings or errors. Why nobody complains about this? (Because we are not in Rust but in C.) > Even when documenting the constraint of immediately using the pointer, > it could easily be overlooked/forgotten. Just like everything else. > It does not even has to be me forgetting the constraint, but could be > someone else changing the code in the future, adding some API call > before the pointer is used, unaware of the possible consequences and > that could invalidate the pointer (if I understand James explanation > correctly). This could go unnoticed easily. Keep away the cat from the code and sit somebody to the chair who used to read the documentation and familiar with the code he modifies. Anyway, if you modify index entries while iterating, then you may miss some entries, or count one twice, who knows. So it is the violation of the API anyways, and you will be happy when you receive an invalid memory access or a fatal av_free() error that will save you days of debugging. > So IMO having a function with many arguments seems like a better and > safer tradeoff in this case to me… Safe-safe... we are still in C, homeland of UB. Your program will not get magically more or less safer depending on what a single FFmpeg API function returns, if you or your colleague (or >me< or >anybody<) never used the standard library before and/or does not read the documentation. Let's investigate a different thing that have not been mentioned before: What if AVIndexEntry changes? Fields ADDED/User INTERESTED: (3): Must use the new av_get_index_entry2(); have to be very careful about the order of parameters. (4): AVIndexEntry.new_field. Fields ADDED/User NOT interested: (3): Deprecation warning for av_get_index_entry(); will have to use function with the new signature, passing NULL, taking care of the
Re: [FFmpeg-devel] [PATCH 3/3] lavfi/dnn_backend_tensorflow.c: fix mem leak in execute_model_tf
> -Original Message- > From: ffmpeg-devel On Behalf Of Ting Fu > Sent: 2021年3月24日 15:39 > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH 3/3] lavfi/dnn_backend_tensorflow.c: fix mem > leak in execute_model_tf > > Signed-off-by: Ting Fu > --- > libavfilter/dnn/dnn_backend_tf.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/libavfilter/dnn/dnn_backend_tf.c > b/libavfilter/dnn/dnn_backend_tf.c > index c18cb4063f..c0aa510630 100644 > --- a/libavfilter/dnn/dnn_backend_tf.c > +++ b/libavfilter/dnn/dnn_backend_tf.c > @@ -766,18 +766,21 @@ static DNNReturnType execute_model_tf(const > DNNModel *model, const char *input_n > if (nb_output != 1) { > // currently, the filter does not need multiple outputs, > // so we just pending the support until we really need it. > +TF_DeleteTensor(input_tensor); > avpriv_report_missing_feature(ctx, "multiple outputs"); > return DNN_ERROR; > } > > tf_outputs = av_malloc_array(nb_output, sizeof(*tf_outputs)); > if (tf_outputs == NULL) { > +TF_DeleteTensor(input_tensor); > av_log(ctx, AV_LOG_ERROR, "Failed to allocate memory for > *tf_outputs\n"); \ > return DNN_ERROR; > } > > output_tensors = av_mallocz_array(nb_output, sizeof(*output_tensors)); > if (!output_tensors) { > +TF_DeleteTensor(input_tensor); > av_freep(&tf_outputs); > av_log(ctx, AV_LOG_ERROR, "Failed to allocate memory for output > tensor\n"); \ > return DNN_ERROR; > @@ -786,6 +789,7 @@ static DNNReturnType execute_model_tf(const > DNNModel *model, const char *input_n > for (int i = 0; i < nb_output; ++i) { > tf_outputs[i].oper = TF_GraphOperationByName(tf_model->graph, > output_names[i]); > if (!tf_outputs[i].oper) { > +TF_DeleteTensor(input_tensor); > av_freep(&tf_outputs); > av_freep(&output_tensors); > av_log(ctx, AV_LOG_ERROR, "Could not find output \"%s\" in > model\n", output_names[i]); \ @@ -799,6 +803,7 @@ static DNNReturnType > execute_model_tf(const DNNModel *model, const char *input_n >tf_outputs, output_tensors, nb_output, >NULL, 0, NULL, tf_model->status); > if (TF_GetCode(tf_model->status) != TF_OK) { > +TF_DeleteTensor(input_tensor); > av_freep(&tf_outputs); > av_freep(&output_tensors); > av_log(ctx, AV_LOG_ERROR, "Failed to run session when executing > model\n"); > -- LGTM, will push soon, thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avformat/utils: Remove redundant check
Andreas Rheinhardt: > This check is outdated because the caller doesn't need to check that > the multiplication overflows when using av_realloc_array() (the code > in question used av_realloc() before that); furthermore, the check > is also a remnant of the time in which our allocation functions > didn't use size_t parameters. > > Signed-off-by: Andreas Rheinhardt > --- > It would btw make more sense for AVFormatContext.max_streams to be > unsigned. > > libavformat/utils.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 524765aeb4..88f6f18f1f 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -4493,9 +4493,10 @@ AVStream *avformat_new_stream(AVFormatContext *s, > const AVCodec *c) > int i; > AVStream **streams; > > -if (s->nb_streams >= FFMIN(s->max_streams, INT_MAX/sizeof(*streams))) { > -if (s->max_streams < INT_MAX/sizeof(*streams)) > -av_log(s, AV_LOG_ERROR, "Number of streams exceeds max_streams > parameter (%d), see the documentation if you wish to increase it\n", > s->max_streams); > +if (s->nb_streams >= s->max_streams) { > +av_log(s, AV_LOG_ERROR, "Number of streams exceeds max_streams > parameter" > + " (%d), see the documentation if you wish to increase it\n", > + s->max_streams); > return NULL; > } > streams = av_realloc_array(s->streams, s->nb_streams + 1, > sizeof(*streams)); > Will apply this patchset tomorrow unless there are objections. - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/sierravmd: Make struct smaller by reordering
Andreas Rheinhardt: > Also remove keyframe from vmd_frame, it is unused. > > Signed-off-by: Andreas Rheinhardt > --- > libavformat/sierravmd.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/libavformat/sierravmd.c b/libavformat/sierravmd.c > index 11a883614f..cfbf1843d4 100644 > --- a/libavformat/sierravmd.c > +++ b/libavformat/sierravmd.c > @@ -38,10 +38,9 @@ > > typedef struct vmd_frame { >int stream_index; > - int64_t frame_offset; >unsigned int frame_size; > + int64_t frame_offset; >int64_t pts; > - int keyframe; >unsigned char frame_record[BYTES_PER_FRAME_RECORD]; > } vmd_frame; > > Will apply this patch tomorrow unless there are objections. - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".