> From: Martin Storsjö <mar...@martin.st> > Sent: Tuesday, April 28, 2020 14:28 > To: Fu, Linjie <linjie...@intel.com> > Cc: FFmpeg development discussions and patches <ffmpeg- > de...@ffmpeg.org> > Subject: RE: [FFmpeg-devel] [PATCH v4 2/9] lavc/libopenh264enc: add > default gop size and bit rate > > On Tue, 28 Apr 2020, Fu, Linjie wrote: > > >> From: Martin Storsjö <mar...@martin.st> > >> Sent: Tuesday, April 28, 2020 03:28 > >>> static const AVCodecDefault svc_enc_defaults[] = { > >>> + { "b", "0" }, > >>> + { "g", "120" }, > >>> { "qmin", "-1" }, > >> > >> Why do you hardcode a value for g here, but put the default bitrate value > >> in the code above? Wouldn't it be clearer to have both defaults here at > >> the same place? > >> > > A default value in svc_enc_defaults[] would help to distinguish between > > "the user specified the bitrate to be <x>" vs. "the user did not specify > anything > > about the target bitrate", as Anton has suggested in [1]. > > > > Considering about your suggestions in patch 1/9, IMO it seems to be more > reasonable > > to keep the uiIntraPeriod untouched. The libopenh264 library would fill the > default > > value of uiIntraPeriod to 0, and as a consequence the gop size would be > rather large. > > > > Updated the default "g" to "-1", same as libx264 did. > > (Note that it's not acceptable for bitrate, since bitrate = 0 as default in > library is not > > valid) > > If we have the option of not setting the bitrate on the encoder, then yes, > it's better to avoid setting one in svc_enc_defaults. But in this case, if > no bitrate was chosen by the user, we end up enforcing a default value > anyway - just that the default is not written in the global defaults table > (libavcodec/options_table.h) and not in svc_enc_defaults, but instead in > code. > > If we actually need to set a bitrate, it's IMO better to put this default
Indeed we have to set the bitrate. > in the table, especially if we have other defaults like gop size there. > In previous discussion in [1], we seems to come to an alignment that this would help to determine whether it's specified by user, and hence allow libopenh264enc to select the RC_MODE through settings like avctx->bit_rate, max/min/avg bitrates if rc_mode is not set explicitly. VAAPI encoder has the similar logic in [2]. I'm okay with either solution, if above logic is not going to be implemented. [1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-April/260215.html [2] https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/vaapi_encode.c#L1542 - Linjie _______________________________________________ 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".