On 25/02/2020 14:10, Fu, Linjie wrote:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
>> Mark Thompson
>> Sent: Monday, February 24, 2020 07:41
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: [FFmpeg-devel] [PATCH v4 17/21] vaapi_encode_h264: Support
>> stereo 3D metadata
>>
>> Insert frame packing arrangement messages into the stream when input
>> frames have associated stereo 3D side-data.
>> ---
>> doc/encoders.texi | 3 +++
>> libavcodec/vaapi_encode_h264.c | 25 ++++++++++++++++++++++++-
>> 2 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/encoders.texi b/doc/encoders.texi
>> index e23b6b32fe..62b6902197 100644
>> --- a/doc/encoders.texi
>> +++ b/doc/encoders.texi
>> @@ -3065,6 +3065,9 @@ Include picture timing parameters
>> (@emph{buffering_period} and
>> @emph{pic_timing} messages).
>> @item recovery_point
>> Include recovery points where appropriate (@emph{recovery_point}
>> messages).
>> +@item frame_packing
>> +Include stereo 3D metadata if the input frames have it
>> +(@emph{frame_packing_arrangement} messages).
>> @end table
>>
>> @end table
>> diff --git a/libavcodec/vaapi_encode_h264.c
>> b/libavcodec/vaapi_encode_h264.c
>> index f4965d8b09..58eae613c4 100644
>> --- a/libavcodec/vaapi_encode_h264.c
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -25,6 +25,7 @@
>> #include "libavutil/common.h"
>> #include "libavutil/internal.h"
>> #include "libavutil/opt.h"
>> +#include "libavutil/stereo3d.h"
>>
>> #include "avcodec.h"
>> #include "cbs.h"
>> @@ -39,6 +40,7 @@ enum {
>> SEI_TIMING = 0x01,
>> SEI_IDENTIFIER = 0x02,
>> SEI_RECOVERY_POINT = 0x04,
>> + SEI_FRAME_PACKING = 0x20,
>> };
>
> There is a jumping from 0x04 to 0x20, how about combining it with the enum in
> vaapi_encode_h265.c, and moving into vaapi_encode.h, hence SEI_FRAME_PACKING
> could also be used for H265 later?
Yeah, the point of including the jump was that they are disjoint parts of the
same enum in the two files.
Moving it into the header would be reasonable, I'll do that (other codecs where
SEI isn't a thing can see it, but they don't care so whatever).
Does anyone use stereo frame packing in H.265? If that's not an entirely
vestigial feature then I would just add it, because it's very easy to do.
>> // Random (version 4) ISO 11578 UUID.
>> @@ -96,6 +98,7 @@ typedef struct VAAPIEncodeH264Context {
>> H264RawSEIBufferingPeriod sei_buffering_period;
>> H264RawSEIPicTiming sei_pic_timing;
>> H264RawSEIRecoveryPoint sei_recovery_point;
>> + H264RawSEIFramePackingArrangement sei_frame_packing;
>> H264RawSEIUserDataUnregistered sei_identifier;
>> char *sei_identifier_string;
>>
>> @@ -251,6 +254,12 @@ static int
>> vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
>> sei->payload[i].payload.recovery_point =
>> priv->sei_recovery_point;
>> ++i;
>> }
>> + if (priv->sei_needed & SEI_FRAME_PACKING) {
>> + sei->payload[i].payload_type = H264_SEI_TYPE_FRAME_PACKING;
>> + sei->payload[i].payload.frame_packing_arrangement =
>> + priv->sei_frame_packing;
>> + ++i;
>> + }
>>
>> sei->payload_count = i;
>> av_assert0(sei->payload_count > 0);
>> @@ -700,6 +709,17 @@ static int
>> vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>> priv->sei_needed |= SEI_RECOVERY_POINT;
>> }
>>
>> + if (priv->sei & SEI_FRAME_PACKING) {
>> + AVFrameSideData *sd = av_frame_get_side_data(pic->input_image,
>> +
>> AV_FRAME_DATA_STEREO3D);
>> + if (sd) {
>> + ff_cbs_h264_fill_sei_frame_packing_arrangement(
>> + &priv->sei_frame_packing, (const AVStereo3D*)sd->data);
>> + }
>> +
>> + priv->sei_needed |= SEI_FRAME_PACKING;
>
> If got NULL sd from av_frame_get_side_data(), would it be better to not
> adding
> SEI_FRAME_PACKING to priv->sei_needed or taking further actions to write
> extra header?
Good point, it needs to be inside the brace. (And I should check negative
cases more carefully.)
Thanks,
- Mark
_______________________________________________
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".
