Re: [FFmpeg-devel] [PATCH 3/3] Adds a new hls_flag avg_bw
Thanks for your feedback. Will soon make these changes and re-submit. -Amit On Fri, Sep 28, 2018 at 7:02 PM Jeyapal, Karthick wrote: > Please find my comments inlined below. > On 9/28/18 11:36 AM, Amit Kale wrote: > > If this flag is set, AVERAGE-BANDWIDTH value will be added to a master > playlist entry. This flag implies peak_segment_bw. > Better to add a code like below to set peak segment bw flag > explicitly(with comments) in hls_init function. > if (hls->flags & HLS_AVG_BW) >hls->flags |= HLS_PEAK_SEGMENT_BW > > > > Signed-off-by: Amit Kale > > --- > > doc/muxers.texi | 4 > > libavformat/dashenc.c | 2 +- > > libavformat/hlsenc.c | 20 > > libavformat/hlsplaylist.c | 6 -- > > libavformat/hlsplaylist.h | 4 ++-- > > 5 files changed, 27 insertions(+), 9 deletions(-) > > > > Index: ffmpeg/doc/muxers.texi > > === > > --- ffmpeg.orig/doc/muxers.texi > > +++ ffmpeg/doc/muxers.texi > > @@ -793,6 +793,10 @@ Possible values: > > If this flag is set, BANDWIDTH value in a master playlist entry will be > > set to the peak segment bandwidth. > > > > +@item avg_bw > Rename this to average_bandwidth > > +If this flag is set, AVERAGE-BANDWIDTH value will be added to a master > > +playlist entry. This flag implies peak_segment_bw. > > + > > @item single_file > > If this flag is set, the muxer will store all segments in a single > MPEG-TS > > file, and will use byte ranges in the playlist. HLS playlists generated > with > > Index: ffmpeg/libavformat/dashenc.c > > === > > --- ffmpeg.orig/libavformat/dashenc.c > > +++ ffmpeg/libavformat/dashenc.c > > @@ -919,7 +919,7 @@ static int write_manifest(AVFormatContex > > av_strlcat(codec_str, audio_codec_str, > sizeof(codec_str)); > > } > > get_hls_playlist_name(playlist_file, sizeof(playlist_file), > NULL, i); > > -ff_hls_write_stream_info(st, out, stream_bitrate, > playlist_file, agroup, > > +ff_hls_write_stream_info(st, out, stream_bitrate, 0, > playlist_file, agroup, > > codec_str, NULL); > > } > > avio_close(out); > > Index: ffmpeg/libavformat/hlsenc.c > > === > > --- ffmpeg.orig/libavformat/hlsenc.c > > +++ ffmpeg/libavformat/hlsenc.c > > @@ -100,6 +100,7 @@ typedef enum HLSFlags { > > HLS_PERIODIC_REKEY = (1 << 12), > > HLS_INDEPENDENT_SEGMENTS = (1 << 13), > > HLS_PEAK_SEGMENT_BW = (1 << 14), > > +HLS_AVG_BW = (1 << 15), > > } HLSFlags; > > > > typedef enum { > > @@ -115,6 +116,8 @@ typedef struct VariantStream { > > AVOutputFormat *vtt_oformat; > > AVIOContext *out; > > int packets_written; > > +int64_t bytes_written; > > +double total_duration; > These can be local variables. It can be calculated during peak bandwidth > calculation loop. > > int init_range_length; > > > > AVFormatContext *avf; > > @@ -1183,7 +1186,7 @@ static int create_master_playlist(AVForm > > AVStream *vid_st, *aud_st; > > AVDictionary *options = NULL; > > unsigned int i, j; > > -int m3u8_name_size, ret, bandwidth; > > +int m3u8_name_size, ret, bandwidth, avgbw; > Could rename it to average_bandwidth for the sake of consistency. > > char *m3u8_rel_name, *ccgroup; > > ClosedCaptionsStream *ccs; > > > > @@ -1303,7 +1306,9 @@ static int create_master_playlist(AVForm > > } > > > > bandwidth = 0; > > -if (last && hls->flags & HLS_PEAK_SEGMENT_BW) { > > +avgbw = 0; > > +if (last && (hls->flags & HLS_PEAK_SEGMENT_BW || > > +hls->flags & HLS_AVG_BW)) { > '|| hls->flags & HLS_AVG_BW' is not required if HLS_PEAK_SEGMENT_BW flag > is set in hls_init as mentioned earlier. > > HLSSegment *hs = vs->segments; > > while (hs) { > > int64_t segment_bandwidth = hs->size * 8 / hs->duration; > vs->bytes_written and vs->total_duration can be made local variables and > its value could be calculated here. > Anyways we are reading hs->size and hs->duration here. > > @@ -1311,6 +1316,8 @@ static int create_master_playlist(AVForm > > bandwidth = segment_bandwidth; > > hs = hs->next; > > } > > +if (hls->flags & HLS_AVG_BW) > > +avgbw = (vs->bytes_written * 8) / vs->total_duration; > > } else { > > if (vid_st) > > bandwidth += get_stream_bit_rate(vid_st); > > @@ -1334,8 +1341,8 @@ static int create_master_playlist(AVForm > > vs->ccgroup); > > } > > > > -ff_hls_write_stream_info(vid_st, hls->m3u8_out, bandwidth, > m3u8_rel_name, > > -aud_st ? vs->agroup : NULL, vs->codec_attr, ccgroup); > > +
[FFmpeg-devel] [PATCH] lavf/utils: change truncating packet log to a warning
Some scene files do this intentionally for the sake of having a nice checksum. --- libavformat/utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/utils.c b/libavformat/utils.c index c973a7e0c5..924b99f6d4 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -257,7 +257,7 @@ int ffio_limit(AVIOContext *s, int size) } if (s->maxsize>= 0 && remaining+1 < size) { -av_log(NULL, remaining ? AV_LOG_ERROR : AV_LOG_DEBUG, "Truncating packet of size %d to %"PRId64"\n", size, remaining+1); +av_log(NULL, remaining ? AV_LOG_WARNING : AV_LOG_DEBUG, "Truncating packet of size %d to %"PRId64"\n", size, remaining+1); size = remaining+1; } } -- 2.19.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/qsvdec:flush the buffered data before reinit
Flush the buffered data in libmfx before video param reinit in case the frames drop. Cache the first frame causing the reinit and decode zero-size pkt to flush the buffered pkt before reinit. After all the buffered pkts being flushed, resume to reinit and decode. Fix the issue in ticket #7399. Modified in qsvdec_other.c as well. Signed-off-by: Linjie Fu Signed-off-by: Zhong Li --- libavcodec/qsvdec.c | 12 libavcodec/qsvdec.h | 2 ++ libavcodec/qsvdec_h2645.c | 11 +++ libavcodec/qsvdec_other.c | 10 +++--- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c index 22e7a46a85..ca4500b456 100644 --- a/libavcodec/qsvdec.c +++ b/libavcodec/qsvdec.c @@ -372,6 +372,8 @@ static int qsv_decode(AVCodecContext *avctx, QSVContext *q, ++q->zero_consume_run; if (q->zero_consume_run > 1) ff_qsv_print_warning(avctx, ret, "A decode call did not consume any data"); +} else if (!*sync && bs.DataOffset) { +++q->buffered_count; } else { q->zero_consume_run = 0; } @@ -493,6 +495,7 @@ int ff_qsv_process_data(AVCodecContext *avctx, QSVContext *q, int dummy_size; int ret; const AVPixFmtDescriptor *desc; +AVPacket zero_pkt = {0}; if (!q->avctx_internal) { q->avctx_internal = avcodec_alloc_context3(NULL); @@ -527,6 +530,15 @@ int ff_qsv_process_data(AVCodecContext *avctx, QSVContext *q, AV_PIX_FMT_NONE }; enum AVPixelFormat qsv_format; +if (q->buffered_count) { +q->reinit_flag = 1; +/* decode zero-size pkt to flush the buffered pkt before reinit */ +q->buffered_count--; +return qsv_decode(avctx, q, frame, got_frame, _pkt); +} + +q->reinit_flag = 0; + qsv_format = ff_qsv_map_pixfmt(q->parser->format, >fourcc); if (qsv_format < 0) { av_log(avctx, AV_LOG_ERROR, diff --git a/libavcodec/qsvdec.h b/libavcodec/qsvdec.h index 5b7b03a48b..111536caba 100644 --- a/libavcodec/qsvdec.h +++ b/libavcodec/qsvdec.h @@ -53,6 +53,8 @@ typedef struct QSVContext { AVFifoBuffer *async_fifo; int zero_consume_run; +int buffered_count; +int reinit_flag; // the internal parser and codec context for parsing the data AVCodecParserContext *parser; diff --git a/libavcodec/qsvdec_h2645.c b/libavcodec/qsvdec_h2645.c index d9d2318d1a..b8a78aa81b 100644 --- a/libavcodec/qsvdec_h2645.c +++ b/libavcodec/qsvdec_h2645.c @@ -146,10 +146,11 @@ static int qsv_decode_frame(AVCodecContext *avctx, void *data, /* no more data */ if (av_fifo_size(s->packet_fifo) < sizeof(AVPacket)) return avpkt->size ? avpkt->size : ff_qsv_process_data(avctx, >qsv, frame, got_frame, avpkt); - -av_packet_unref(>buffer_pkt); - -av_fifo_generic_read(s->packet_fifo, >buffer_pkt, sizeof(s->buffer_pkt), NULL); +/* in progress of reinit, no read from fifo and keep the buffer_pkt */ +if (!s->qsv.reinit_flag) { +av_packet_unref(>buffer_pkt); +av_fifo_generic_read(s->packet_fifo, >buffer_pkt, sizeof(s->buffer_pkt), NULL); +} } ret = ff_qsv_process_data(avctx, >qsv, frame, got_frame, >buffer_pkt); @@ -159,6 +160,8 @@ static int qsv_decode_frame(AVCodecContext *avctx, void *data, av_packet_unref(>buffer_pkt); return ret; } +if (s->qsv.reinit_flag) +continue; s->buffer_pkt.size -= ret; s->buffer_pkt.data += ret; diff --git a/libavcodec/qsvdec_other.c b/libavcodec/qsvdec_other.c index 993c7a8e80..b449bb21b7 100644 --- a/libavcodec/qsvdec_other.c +++ b/libavcodec/qsvdec_other.c @@ -132,9 +132,11 @@ static int qsv_decode_frame(AVCodecContext *avctx, void *data, /* no more data */ if (av_fifo_size(s->packet_fifo) < sizeof(AVPacket)) return avpkt->size ? avpkt->size : ff_qsv_process_data(avctx, >qsv, frame, got_frame, avpkt); - -av_packet_unref(>input_ref); -av_fifo_generic_read(s->packet_fifo, >input_ref, sizeof(s->input_ref), NULL); +/* in progress of reinit, no read from fifo and keep the buffer_pkt */ +if (!s->qsv.reinit_flag) { +av_packet_unref(>input_ref); +av_fifo_generic_read(s->packet_fifo, >input_ref, sizeof(s->input_ref), NULL); +} } ret = ff_qsv_process_data(avctx, >qsv, frame, got_frame, >input_ref); @@ -145,6 +147,8 @@ static int qsv_decode_frame(AVCodecContext *avctx, void *data, return ret; } +if (s->qsv.reinit_flag) +continue; s->input_ref.size -= ret; s->input_ref.data += ret; -- 2.17.1 ___ ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavc/hevc: Don't parse NAL unit for a dummy buffer
On Sat, Sep 29, 2018 at 03:11:40AM +, Li, Zhong wrote: > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > > Of myp...@gmail.com > > Sent: Friday, September 28, 2018 9:51 AM > > To: FFmpeg development discussions and patches > > > > Cc: l...@chinaffmpeg.org; s...@jkqxz.net > > Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/hevc: Don't parse NAL unit for a > > dummy buffer > > > > On Fri, Apr 13, 2018 at 9:04 AM Xiang, Haihao > > wrote: > > > > > > > > > Thank Steven for reviewing the patch, could anyone help to push the > > patch? > > > > > > Best Regards > > > Haihao > > > > > > > > On 8 Apr 2018, at 12:53, Xiang, Haihao > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Steven, > > > > > > > > > > Are there more comments on this patch? > > > > > > > > > > Thanks > > > > > Haihao > > > > > > > > > > > > > > > > hevc parser mistakenly reports the following message if a dummy > > > > > > buffer is padded for EOF > > > > > > > > > > > > [hevc @ 0x559b63848610] missing picture in access unit > > > > > > > > > > > > v2: use the preferred code style and rebase the patch > > > > > > > > > > > > Signed-off-by: Haihao Xiang > > > > > > --- > > > > > > libavcodec/hevc_parser.c | 7 ++- > > > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c > > > > > > index a468682ed3..01418b276d 100644 > > > > > > --- a/libavcodec/hevc_parser.c > > > > > > +++ b/libavcodec/hevc_parser.c > > > > > > @@ -294,6 +294,8 @@ static int hevc_parse(AVCodecParserContext > > > > > > *s, AVCodecContext *avctx, > > > > > > int next; > > > > > > HEVCParserContext *ctx = s->priv_data; > > > > > > ParseContext *pc = >pc; > > > > > > +int is_dummy_buf = !buf_size; > > > > > > +const uint8_t *dummy_buf = buf; > > > > > > > > > > > > if (avctx->extradata && !ctx->parsed_extradata) { > > > > > > ff_hevc_decode_extradata(avctx->extradata, > > > > > > avctx->extradata_size, >ps, >sei, @@ -313,7 +315,10 @@ > > > > > > static int hevc_parse(AVCodecParserContext *s, AVCodecContext > > > > > > *avctx, > > > > > > } > > > > > > } > > > > > > > > > > > > -parse_nal_units(s, buf, buf_size, avctx); > > > > > > +is_dummy_buf = (is_dummy_buf && (dummy_buf == buf)); > > > > > > + > > > > > > +if (!is_dummy_buf) > > > > > > +parse_nal_units(s, buf, buf_size, avctx); > > > > > > > > > > > > *poutbuf = buf; > > > > > > *poutbuf_size = buf_size; > > > > > > > > LGTM > > > > > > > > Thanks > > > > Steven > > > > > > > > > > > > > > I've encountered this annoying error message, can we push the patch? > > Thanks. > > > > BTW: The dummy_buf is not part of the original HEVC clip, it's come from > > av_parser_parse2() if buf_size == 0. > > > > Thanks. > > Patch LGTM. will apply thx [...] -- 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 http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2 1/7] avformat/ip: factorize some IP filtering and resolving functions to a new file
On Mon, 24 Sep 2018, Michael Niedermayer wrote: On Mon, Sep 24, 2018 at 08:49:27AM +0200, Marton Balint wrote: These are based on the very similar UDP and RTP protocol functions. Signed-off-by: Marton Balint --- libavformat/ip.c | 159 +++ libavformat/ip.h | 72 + 2 files changed, 231 insertions(+) create mode 100644 libavformat/ip.c create mode 100644 libavformat/ip.h [...] + * Parses the address[,address] source block list in buf and adds it to the + * filters in the IPSourceFilters structure. + * @return 0 on success, < 0 AVERROR code on error. + */ +int ff_ip_parse_blocks(void *log_ctx, const char *buf, IPSourceFilters *filters); Thanks Ping for the whole series... Thanks, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avcodec/libzvbi-teletextdec: formatted ass output
On Thu, 17 May 2018, Aman Gupta wrote: On Sun, May 6, 2018 at 2:05 PM, Marton Balint wrote: Inspired by the VideoLAN text decoder and its port to FFmpeg made by Aman Gupta. This got kind of forgotten, but now I pushed it after a rebase and a minor fix. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On Sat, 29 Sep 2018 at 15:19, James Almer wrote: > On 9/29/2018 10:14 AM, Hendrik Leppkes wrote: > > On Sat, Sep 29, 2018 at 3:02 PM Devin Heitmueller > > wrote: > >> > >> On Sat, Sep 29, 2018 at 6:04 AM Rostislav Pehlivanov > >> wrote: > >>> I'd much rather go with the original intent which was to merge the > decoder > >>> into lavc. > >> > >> Ronald can correct me if I'm wrong, but I suspect a key goal behind > >> the decoder was to have a standalone library which could be shared > >> across a variety of projects (both open and closed source). Merging > >> it in directly will create a maintenance headache as the two source > >> trees diverge. It also makes unit testing more difficult, since > >> Ronald/VideoLAN can write unit tests for the library (which will > >> presumably be consumed by a number of projects/products) and be > >> confident that those same unit tests apply to the version that is used > >> by ffmpeg. > >> > >> I don't think having libx264/libx265 out of tree hasn't been a > >> nightmare for anyone. I don't think this case would be any different. > >> > > > > Decoders and encoders are quite a different type of beast. For one > > decoders are ultimately "finished" at some point, where the number of > > changes drastically reduces over time, and decoding is the bread and > > butter of avcodec, which would give immediate av1 support to hundreds > > if not thousands of applications out there, without any further > > developer intervention. > > I'm guessing LGPL is a deterrent for some potential users VideoLAN > wanted for this decoder, hence the 2-Clause BSD license in libdav1d. > On the other hand the 2-Clause BSD is a deterrent for me to write some assembly and just generally pour work on it. > Additionally, I can already name an important key area where not > > having an internal decoder will be a nightmare: hardware decoding. > > Our hardware decoding framework heavily relies on having a decoder > > that does all the header parsing, frame management, and whatnot, and > > the hardware acceleration hooks just by-pass the actual slice decoding > > then. If we don't have an internal decoder, we would have to either > > build half decoder just to hook up hwaccel, or dav1d would have to > > give us extremely detailed bitstream information and slice-level > > bitstream hooks, which I don't think is very practical either. > > > > A new mainstream codec without hardware acceleration support is not > > going to make it, no matter how fast you make the decoder. > > Yeah, this is one of the main reasons to have it as an internal decoder. > But considering actual hardware capable of decoding AV1 will not come > out until like late 2019 at the earliest, there's time for libdav1d to > mature before porting it becomes a must. > Considering the code style and some parts of the internal API are quite close I reckon the wrapper won't survive for more than a few months, and will definitely be gone by January. I'd agree to the wrapper if it didn't make it to 4.1, since it won't be in 4.2. No point in adding a non-default decoder that users may need to specify (not sure how precedence works with both libaomdec and libdav1id). Especially knowing how long cargo cult command lines and API usage lasts. At some point it will be "complete" and development pretty much halted, > much like vp9 currently is (not counting some missing assembly), so the > argument about extra maintenance of separate codebases will not apply > anymore. > Given the license difference, the preference of everyone involved with it for LGPL (its only BSD and a separate library because money) I think chances are people/companies will sooner abandon it to work on their fork than use the base version as-is (or in case of companies contribute). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On Sat, Sep 29, 2018 at 5:46 PM Jean-Baptiste Kempf wrote: > > On Sat, 29 Sep 2018, at 17:32, Hendrik Leppkes wrote: > > On Sat, Sep 29, 2018 at 4:37 PM Jean-Baptiste Kempf > > wrote: > > > > > > On Sat, 29 Sep 2018, at 15:14, Hendrik Leppkes wrote: > > > > I'm still a bit annoyed that this wasn't even a consideration when it > > > > was > > > > > > How do you know it was not considered? > > > > I took BBB by his word when he said so. > > You really think that the person that wrote ffvp9 (and merged it inside > libavcodec) and a large part of dav1d, and who is a large contributor to > FFmpeg, did not consider whether it was better to start inside libavcodec or > as an external library? > Just to make sure, I was specifically talking about hardware acceleration (which my entire post was more or less about), which according to Ronald was not considered in the reasoning here. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On Sat, 29 Sep 2018, at 17:32, Hendrik Leppkes wrote: > On Sat, Sep 29, 2018 at 4:37 PM Jean-Baptiste Kempf wrote: > > > > On Sat, 29 Sep 2018, at 15:14, Hendrik Leppkes wrote: > > > I'm still a bit annoyed that this wasn't even a consideration when it was > > > > How do you know it was not considered? > > I took BBB by his word when he said so. You really think that the person that wrote ffvp9 (and merged it inside libavcodec) and a large part of dav1d, and who is a large contributor to FFmpeg, did not consider whether it was better to start inside libavcodec or as an external library? Come on... -- Jean-Baptiste Kempf - President +33 672 704 734 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On Sat, Sep 29, 2018 at 4:37 PM Jean-Baptiste Kempf wrote: > > On Sat, 29 Sep 2018, at 15:14, Hendrik Leppkes wrote: > > I'm still a bit annoyed that this wasn't even a consideration when it was > > How do you know it was not considered? I took BBB by his word when he said so. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On 9/29/18, Jean-Baptiste Kempf wrote: > On Sat, 29 Sep 2018, at 15:14, Hendrik Leppkes wrote: >> I'm still a bit annoyed that this wasn't even a consideration when it was > > How do you know it was not considered? > Knowing that most developers are part of the FFmpeg/x264/VLC community, you > can be sure it was considered. > >> decided to move it into a separate library, but its not like they actually >> asked for arguments. > > Arguments have been asked to numerous people, who just might have a > different opinion that yours. > > Also, it was not 'moved' to a separate library, but created elsewhere. Was I asked? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On Sat, 29 Sep 2018, at 15:14, Hendrik Leppkes wrote: > I'm still a bit annoyed that this wasn't even a consideration when it was How do you know it was not considered? Knowing that most developers are part of the FFmpeg/x264/VLC community, you can be sure it was considered. > decided to move it into a separate library, but its not like they actually > asked for arguments. Arguments have been asked to numerous people, who just might have a different opinion that yours. Also, it was not 'moved' to a separate library, but created elsewhere. -- Jean-Baptiste Kempf - President +33 672 704 734 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On 9/29/2018 10:14 AM, Hendrik Leppkes wrote: > On Sat, Sep 29, 2018 at 3:02 PM Devin Heitmueller > wrote: >> >> On Sat, Sep 29, 2018 at 6:04 AM Rostislav Pehlivanov >> wrote: >>> I'd much rather go with the original intent which was to merge the decoder >>> into lavc. >> >> Ronald can correct me if I'm wrong, but I suspect a key goal behind >> the decoder was to have a standalone library which could be shared >> across a variety of projects (both open and closed source). Merging >> it in directly will create a maintenance headache as the two source >> trees diverge. It also makes unit testing more difficult, since >> Ronald/VideoLAN can write unit tests for the library (which will >> presumably be consumed by a number of projects/products) and be >> confident that those same unit tests apply to the version that is used >> by ffmpeg. >> >> I don't think having libx264/libx265 out of tree hasn't been a >> nightmare for anyone. I don't think this case would be any different. >> > > Decoders and encoders are quite a different type of beast. For one > decoders are ultimately "finished" at some point, where the number of > changes drastically reduces over time, and decoding is the bread and > butter of avcodec, which would give immediate av1 support to hundreds > if not thousands of applications out there, without any further > developer intervention. I'm guessing LGPL is a deterrent for some potential users VideoLAN wanted for this decoder, hence the 2-Clause BSD license in libdav1d. > > Additionally, I can already name an important key area where not > having an internal decoder will be a nightmare: hardware decoding. > Our hardware decoding framework heavily relies on having a decoder > that does all the header parsing, frame management, and whatnot, and > the hardware acceleration hooks just by-pass the actual slice decoding > then. If we don't have an internal decoder, we would have to either > build half decoder just to hook up hwaccel, or dav1d would have to > give us extremely detailed bitstream information and slice-level > bitstream hooks, which I don't think is very practical either. > > A new mainstream codec without hardware acceleration support is not > going to make it, no matter how fast you make the decoder. Yeah, this is one of the main reasons to have it as an internal decoder. But considering actual hardware capable of decoding AV1 will not come out until like late 2019 at the earliest, there's time for libdav1d to mature before porting it becomes a must. At some point it will be "complete" and development pretty much halted, much like vp9 currently is (not counting some missing assembly), so the argument about extra maintenance of separate codebases will not apply anymore. And to be honest the extra maintenance argument is not too strong to begin with, considering all the time lost having to NIH a lot of standard features already available in libavutil, like memory allocator wrappers and reference counted buffers, or muxer/demuxers to use with their CLI, all already available in libavformat, because they can't link to either of them. > I'm still a > bit annoyed that this wasn't even a consideration when it was decided > to move it into a separate library, but its not like they actually > asked for arguments. > > - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On 9/29/18, Devin Heitmueller wrote: > On Sat, Sep 29, 2018 at 6:04 AM Rostislav Pehlivanov > wrote: >> I'd much rather go with the original intent which was to merge the decoder >> into lavc. > > Ronald can correct me if I'm wrong, but I suspect a key goal behind > the decoder was to have a standalone library which could be shared > across a variety of projects (both open and closed source). Merging > it in directly will create a maintenance headache as the two source > trees diverge. It also makes unit testing more difficult, since > Ronald/VideoLAN can write unit tests for the library (which will > presumably be consumed by a number of projects/products) and be > confident that those same unit tests apply to the version that is used > by ffmpeg. > > I don't think having libx264/libx265 out of tree hasn't been a > nightmare for anyone. I don't think this case would be any different. > I simply disagree with you. Encoders are different thing. We need native decoders. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On Sat, Sep 29, 2018 at 3:02 PM Devin Heitmueller wrote: > > On Sat, Sep 29, 2018 at 6:04 AM Rostislav Pehlivanov > wrote: > > I'd much rather go with the original intent which was to merge the decoder > > into lavc. > > Ronald can correct me if I'm wrong, but I suspect a key goal behind > the decoder was to have a standalone library which could be shared > across a variety of projects (both open and closed source). Merging > it in directly will create a maintenance headache as the two source > trees diverge. It also makes unit testing more difficult, since > Ronald/VideoLAN can write unit tests for the library (which will > presumably be consumed by a number of projects/products) and be > confident that those same unit tests apply to the version that is used > by ffmpeg. > > I don't think having libx264/libx265 out of tree hasn't been a > nightmare for anyone. I don't think this case would be any different. > Decoders and encoders are quite a different type of beast. For one decoders are ultimately "finished" at some point, where the number of changes drastically reduces over time, and decoding is the bread and butter of avcodec, which would give immediate av1 support to hundreds if not thousands of applications out there, without any further developer intervention. Additionally, I can already name an important key area where not having an internal decoder will be a nightmare: hardware decoding. Our hardware decoding framework heavily relies on having a decoder that does all the header parsing, frame management, and whatnot, and the hardware acceleration hooks just by-pass the actual slice decoding then. If we don't have an internal decoder, we would have to either build half decoder just to hook up hwaccel, or dav1d would have to give us extremely detailed bitstream information and slice-level bitstream hooks, which I don't think is very practical either. A new mainstream codec without hardware acceleration support is not going to make it, no matter how fast you make the decoder. I'm still a bit annoyed that this wasn't even a consideration when it was decided to move it into a separate library, but its not like they actually asked for arguments. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On 9/29/2018 9:54 AM, Devin Heitmueller wrote: > On Sat, Sep 29, 2018 at 6:04 AM Rostislav Pehlivanov > wrote: >> I'd much rather go with the original intent which was to merge the decoder >> into lavc. > > Ronald can correct me if I'm wrong, but I suspect a key goal behind > the decoder was to have a standalone library which could be shared > across a variety of projects (both open and closed source). Merging > it in directly will create a maintenance headache as the two source > trees diverge. It also makes unit testing more difficult, since > Ronald/VideoLAN can write unit tests for the library (which will > presumably be consumed by a number of projects/products) and be > confident that those same unit tests apply to the version that is used > by ffmpeg. > > I don't think having libx264/libx265 out of tree hasn't been a > nightmare for anyone. I don't think this case would be any different. > > Devin There's a considerable difference between an encoder and a decoder from a ffmpeg PoV. For starters, hwaccel hooks, and then threading is instead handled by the library. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On Sat, Sep 29, 2018 at 6:04 AM Rostislav Pehlivanov wrote: > I'd much rather go with the original intent which was to merge the decoder > into lavc. Ronald can correct me if I'm wrong, but I suspect a key goal behind the decoder was to have a standalone library which could be shared across a variety of projects (both open and closed source). Merging it in directly will create a maintenance headache as the two source trees diverge. It also makes unit testing more difficult, since Ronald/VideoLAN can write unit tests for the library (which will presumably be consumed by a number of projects/products) and be confident that those same unit tests apply to the version that is used by ffmpeg. I don't think having libx264/libx265 out of tree hasn't been a nightmare for anyone. I don't think this case would be any different. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On 9/29/2018 8:05 AM, Hendrik Leppkes wrote: > On Sat, Sep 29, 2018 at 12:05 PM Rostislav Pehlivanov > wrote: >> >> On Fri, 28 Sep 2018 at 22:51, James Almer wrote: >> >>> From: Ronald S. Bultje >>> >>> Originally written by Ronald S. Bultje, with fixes, optimizations and >>> improvements by James Almer. >>> >>> Signed-off-by: James Almer >>> >> >> I'd much rather go with the original intent which was to merge the decoder >> into lavc. >> We already have libaomdec which is fast-ish but works as a stop-gap >> solution until we do that. > > I would definitely support that. Not having an internal decoder for > av1 would be a real pain. > > - Hendrik I want the same, but until that happens this wrapper wont hurt, much like the one we added for libdcadec a few years ago before it was ported into an internal one. If we don't add this wrapper in the meantime, people will start asking us about it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On Sat, Sep 29, 2018 at 12:05 PM Rostislav Pehlivanov wrote: > > On Fri, 28 Sep 2018 at 22:51, James Almer wrote: > > > From: Ronald S. Bultje > > > > Originally written by Ronald S. Bultje, with fixes, optimizations and > > improvements by James Almer. > > > > Signed-off-by: James Almer > > > > I'd much rather go with the original intent which was to merge the decoder > into lavc. > We already have libaomdec which is fast-ish but works as a stop-gap > solution until we do that. I would definitely support that. Not having an internal decoder for av1 would be a real pain. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: libdav1d AV1 decoder wrapper
On Fri, 28 Sep 2018 at 22:51, James Almer wrote: > From: Ronald S. Bultje > > Originally written by Ronald S. Bultje, with fixes, optimizations and > improvements by James Almer. > > Signed-off-by: James Almer > I'd much rather go with the original intent which was to merge the decoder into lavc. We already have libaomdec which is fast-ish but works as a stop-gap solution until we do that. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH V3] avcodec/h264_mp4toannexb_bsf: extract extradata for first coming frame
On 29/09/18 08:43, Linjie Fu wrote: > Add "new_nal_slice" to indicate the first coming nal_slice (including > H264_NAL_IDR_SLICE and H264_NAL_SLICE) > > Extract extradata of streams when the first nal_slice comes, not only > H264_NAL_IDR_SLICE but also H264_NAL_SLICE. > > This patch aims at the following issues: > 1. the IDR frame is missing in the first GOP of a stream > (common in live stream, IDR.No.Inband.SPPS.mkv in ticket #6418) > 2. there is no IDR frame in the input stream. > (No.Inband.SPPS.No.IDR.mkv in ticket #6418) > > Both clips could be decoded successfully using software decoding > but had problems in hwaccel using qsv before applying the patch. > > V3: modified the commit message and the code duplication. > > Signed-off-by: Linjie Fu > --- > libavcodec/h264_mp4toannexb_bsf.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/h264_mp4toannexb_bsf.c > b/libavcodec/h264_mp4toannexb_bsf.c > index fb3f24ea40..8dd9159b22 100644 > --- a/libavcodec/h264_mp4toannexb_bsf.c > +++ b/libavcodec/h264_mp4toannexb_bsf.c > @@ -33,6 +33,7 @@ typedef struct H264BSFContext { > int32_t pps_offset; > uint8_t length_size; > uint8_t new_idr; > +uint8_t new_nal_slice; > uint8_t idr_sps_seen; > uint8_t idr_pps_seen; > int extradata_parsed; > @@ -236,13 +237,17 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, > AVPacket *out) > if (!s->new_idr && unit_type == H264_NAL_IDR_SLICE && (buf[1] & > 0x80)) > s->new_idr = 1; > > -/* prepend only to the first type 5 NAL unit of an IDR picture, if > no sps/pps are already present */ > -if (s->new_idr && unit_type == H264_NAL_IDR_SLICE && > !s->idr_sps_seen && !s->idr_pps_seen) { > +/* prepend to the first type 5 NAL unit of an IDR picture, > + * or the first type 1 NAL unit of an IDR missing picture, > + * if no sps/pps are already present */ > +if (s->new_idr && !s->idr_sps_seen && !s->idr_pps_seen > +&& (unit_type == H264_NAL_IDR_SLICE || > (!s->new_nal_slice && H264_NAL_SLICE == unit_type))) { > if ((ret=alloc_and_copy(out, > ctx->par_out->extradata, > ctx->par_out->extradata_size, > buf, nal_size, 1)) < 0) > goto fail; > s->new_idr = 0; > +s->new_nal_slice = 1; > /* if only SPS has been seen, also insert PPS */ > } else if (s->new_idr && unit_type == H264_NAL_IDR_SLICE && > s->idr_sps_seen && !s->idr_pps_seen) { > if (s->pps_offset == -1) { > I'm not really against this change, but it's hard to be sure it doesn't perturb something else because the h264_mp4toannexb BSF gets used in a lot of places. It would seem safer to only do this on real recovery points, rather than blindly splicing it in front of any frame? The frame you actually put it with here is still unlikely to be usefully decoable if you cut a live stream at a random point. As an alternative for your libmfx case, most of the other decoders using h264_mp4toannexb (CrystalHD, CUVID, Mediacodec, RKMPP; not V4L2M2M) consume the extradata separately rather than requiring it to be in the stream - if libmfx could do that then it would already be available and there wouldn't be a need to splice it into the stream in a questionable place like this. (Or, since the normal decoder is fine, just use the appropriate hwaccel (+ hwmap for libmfx output if wanted) rather than the legacy libmfx decoders.) Thanks, - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH V3] avcodec/h264_mp4toannexb_bsf: extract extradata for first coming frame
Add "new_nal_slice" to indicate the first coming nal_slice (including H264_NAL_IDR_SLICE and H264_NAL_SLICE) Extract extradata of streams when the first nal_slice comes, not only H264_NAL_IDR_SLICE but also H264_NAL_SLICE. This patch aims at the following issues: 1. the IDR frame is missing in the first GOP of a stream (common in live stream, IDR.No.Inband.SPPS.mkv in ticket #6418) 2. there is no IDR frame in the input stream. (No.Inband.SPPS.No.IDR.mkv in ticket #6418) Both clips could be decoded successfully using software decoding but had problems in hwaccel using qsv before applying the patch. V3: modified the commit message and the code duplication. Signed-off-by: Linjie Fu --- libavcodec/h264_mp4toannexb_bsf.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libavcodec/h264_mp4toannexb_bsf.c b/libavcodec/h264_mp4toannexb_bsf.c index fb3f24ea40..8dd9159b22 100644 --- a/libavcodec/h264_mp4toannexb_bsf.c +++ b/libavcodec/h264_mp4toannexb_bsf.c @@ -33,6 +33,7 @@ typedef struct H264BSFContext { int32_t pps_offset; uint8_t length_size; uint8_t new_idr; +uint8_t new_nal_slice; uint8_t idr_sps_seen; uint8_t idr_pps_seen; int extradata_parsed; @@ -236,13 +237,17 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out) if (!s->new_idr && unit_type == H264_NAL_IDR_SLICE && (buf[1] & 0x80)) s->new_idr = 1; -/* prepend only to the first type 5 NAL unit of an IDR picture, if no sps/pps are already present */ -if (s->new_idr && unit_type == H264_NAL_IDR_SLICE && !s->idr_sps_seen && !s->idr_pps_seen) { +/* prepend to the first type 5 NAL unit of an IDR picture, + * or the first type 1 NAL unit of an IDR missing picture, + * if no sps/pps are already present */ +if (s->new_idr && !s->idr_sps_seen && !s->idr_pps_seen +&& (unit_type == H264_NAL_IDR_SLICE || (!s->new_nal_slice && H264_NAL_SLICE == unit_type))) { if ((ret=alloc_and_copy(out, ctx->par_out->extradata, ctx->par_out->extradata_size, buf, nal_size, 1)) < 0) goto fail; s->new_idr = 0; +s->new_nal_slice = 1; /* if only SPS has been seen, also insert PPS */ } else if (s->new_idr && unit_type == H264_NAL_IDR_SLICE && s->idr_sps_seen && !s->idr_pps_seen) { if (s->pps_offset == -1) { -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel