| Message ID | 20200403130454.25228-2-lance.lmwang@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,v1,1/2] avcodec/mpeg12enc: Use FF_PROFILE_MPEG2_xxx macros | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Fri, Apr 3, 2020 at 3:12 PM <lance.lmwang@gmail.com> wrote: > > From: Limin Wang <lance.lmwang@gmail.com> > > make setting profile more user friendly > Please make sure API users can still set it through avctx->profile, otherwise this would be an API break. - Hendrik
On Fri, Apr 03, 2020 at 04:25:15PM +0200, Hendrik Leppkes wrote: > On Fri, Apr 3, 2020 at 3:12 PM <lance.lmwang@gmail.com> wrote: > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > make setting profile more user friendly > > > > Please make sure API users can still set it through avctx->profile, > otherwise this would be an API break. Right, I'll update the patch to check avctx->profile != unknown, then overwrite the s->profile with it to void API break. > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Limin Wang > Sent: Friday, April 3, 2020 22:55 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v1 2/2] avcodec/mpeg12enc: Support > mpeg2 encoder profile with const options > > On Fri, Apr 03, 2020 at 04:25:15PM +0200, Hendrik Leppkes wrote: > > On Fri, Apr 3, 2020 at 3:12 PM <lance.lmwang@gmail.com> wrote: > > > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > > > make setting profile more user friendly > > > > > > > Please make sure API users can still set it through avctx->profile, > > otherwise this would be an API break. > > Right, I'll update the patch to check avctx->profile != unknown, then > overwrite > the s->profile with it to void API break. > Would it be better to determine the profile with a priority like: 1.s->profile; then 2. avctx->profile; then 3. a default profile; in case both avctx->profile and s->profile are set? If user set a profile explicitly (through either option in cmdline or av_opt_set()), above priority seems to be natural. (and libx264 [1] has the same logic) - Linjie [1] https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/libx264.c#L806
On Fri, Apr 3, 2020 at 5:47 PM Fu, Linjie <linjie.fu@intel.com> wrote: > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Limin Wang > > Sent: Friday, April 3, 2020 22:55 > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH v1 2/2] avcodec/mpeg12enc: Support > > mpeg2 encoder profile with const options > > > > On Fri, Apr 03, 2020 at 04:25:15PM +0200, Hendrik Leppkes wrote: > > > On Fri, Apr 3, 2020 at 3:12 PM <lance.lmwang@gmail.com> wrote: > > > > > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > > > > > make setting profile more user friendly > > > > > > > > > > Please make sure API users can still set it through avctx->profile, > > > otherwise this would be an API break. > > > > Right, I'll update the patch to check avctx->profile != unknown, then > > overwrite > > the s->profile with it to void API break. > > > Would it be better to determine the profile with a priority like: > 1.s->profile; then > 2. avctx->profile; then > 3. a default profile; > in case both avctx->profile and s->profile are set? > > If user set a profile explicitly (through either option in cmdline or av_opt_set()), > above priority seems to be natural. (and libx264 [1] has the same logic) > That would be just fine. - Hendrik
On Fri, Apr 03, 2020 at 03:47:09PM +0000, Fu, Linjie wrote: > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Limin Wang > > Sent: Friday, April 3, 2020 22:55 > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH v1 2/2] avcodec/mpeg12enc: Support > > mpeg2 encoder profile with const options > > > > On Fri, Apr 03, 2020 at 04:25:15PM +0200, Hendrik Leppkes wrote: > > > On Fri, Apr 3, 2020 at 3:12 PM <lance.lmwang@gmail.com> wrote: > > > > > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > > > > > make setting profile more user friendly > > > > > > > > > > Please make sure API users can still set it through avctx->profile, > > > otherwise this would be an API break. > > > > Right, I'll update the patch to check avctx->profile != unknown, then > > overwrite > > the s->profile with it to void API break. > > > Would it be better to determine the profile with a priority like: > 1.s->profile; then > 2. avctx->profile; then > 3. a default profile; > in case both avctx->profile and s->profile are set? > > If user set a profile explicitly (through either option in cmdline or av_opt_set()), > above priority seems to be natural. (and libx264 [1] has the same logic) I'm OK with the logic, will update the patch change the priority. thx. > > - Linjie > > [1] https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/libx264.c#L806 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/doc/encoders.texi b/doc/encoders.texi index e23b6b32fe..5022b9407e 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -2753,6 +2753,14 @@ For maximum compatibility, use @samp{component}. @item a53cc @var{boolean} Import closed captions (which must be ATSC compatible format) into output. Default is 1 (on). +@item profile @var{integer} +Select the mpeg2 profile to encode, possible values: +@table @samp +@item 422 +@item high +@item main +@item simple +@end table @end table @section png diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c index 643ba8165a..ef8757e5b7 100644 --- a/libavcodec/mpeg12enc.c +++ b/libavcodec/mpeg12enc.c @@ -156,23 +156,27 @@ static av_cold int encode_init(AVCodecContext *avctx) } } - if (avctx->profile == FF_PROFILE_UNKNOWN) { + if (s->profile == FF_PROFILE_UNKNOWN) { if (avctx->level != FF_LEVEL_UNKNOWN) { av_log(avctx, AV_LOG_ERROR, "Set profile and level\n"); return -1; } /* Main or 4:2:2 */ avctx->profile = s->chroma_format == CHROMA_420 ? FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422; + s->profile = s->chroma_format == CHROMA_420 ? FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422; + } else if (s->profile < FF_PROFILE_MPEG2_422) { + av_log(avctx, AV_LOG_ERROR, "Invalid mpeg2 profile set\n"); + return -1; } if (avctx->level == FF_LEVEL_UNKNOWN) { - if (avctx->profile == FF_PROFILE_MPEG2_422) { /* 4:2:2 */ + if (s->profile == FF_PROFILE_MPEG2_422) { /* 4:2:2 */ if (avctx->width <= 720 && avctx->height <= 608) avctx->level = 5; /* Main */ else avctx->level = 2; /* High */ } else { - if (avctx->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format != CHROMA_420) { + if (s->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format != CHROMA_420) { av_log(avctx, AV_LOG_ERROR, "Only High(1) and 4:2:2(0) profiles support 4:2:2 color sampling\n"); return -1; @@ -321,9 +325,9 @@ static void mpeg1_encode_sequence_header(MpegEncContext *s) put_header(s, EXT_START_CODE); put_bits(&s->pb, 4, 1); // seq ext - put_bits(&s->pb, 1, s->avctx->profile == FF_PROFILE_MPEG2_422); // escx 1 for 4:2:2 profile + put_bits(&s->pb, 1, s->profile == FF_PROFILE_MPEG2_422); // escx 1 for 4:2:2 profile - put_bits(&s->pb, 3, s->avctx->profile); // profile + put_bits(&s->pb, 3, s->profile); // profile put_bits(&s->pb, 4, s->avctx->level); // level put_bits(&s->pb, 1, s->progressive_sequence); @@ -1165,6 +1169,11 @@ static const AVOption mpeg2_options[] = { { "secam", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = VIDEO_FORMAT_SECAM }, 0, 0, VE, "video_format" }, { "mac", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = VIDEO_FORMAT_MAC }, 0, 0, VE, "video_format" }, { "unspecified", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = VIDEO_FORMAT_UNSPECIFIED}, 0, 0, VE, "video_format" }, + { "profile", "Set the profile", OFFSET(profile), AV_OPT_TYPE_INT,{ .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, FF_PROFILE_MPEG2_SIMPLE, VE, "profile" }, + { "422", "", 0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_422 }, 0, 0, VE, "profile" }, + { "high", "", 0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_HIGH }, 0, 0, VE, "profile" }, + { "main", "", 0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_MAIN }, 0, 0, VE, "profile" }, + { "simple", "", 0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_SIMPLE }, 0, 0, VE, "profile" }, FF_MPV_COMMON_OPTS { NULL }, }; diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h index 29e692f245..cee423eea1 100644 --- a/libavcodec/mpegvideo.h +++ b/libavcodec/mpegvideo.h @@ -456,6 +456,7 @@ typedef struct MpegEncContext { int progressive_sequence; int mpeg_f_code[2][2]; int a53_cc; + int profile; // picture structure defines are loaded from mpegutils.h int picture_structure;