On Tue, 28 Apr 2020, Fu, Linjie wrote:
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.
Right, so if logic that selects rc mode based on those settings will be
implemented, then yes it makes sense to keep the default at -1 and fall
back to a default bitrate within code. In that case I'd kind of prefer to
have the default bitrate specified in a define high up in the file,
instead of buried in init logic though.
// Martin
_______________________________________________
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".
