Re: [FFmpeg-devel] [PATCH 1/3] avcodec/av1_metadata: filter parameter sets in packet side data
On 4/22/2020 9:37 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Extradata included in packet side data is meant to replace the codec context
>> extradata. So when muxing for example to MP4 without this change and if
>> extradata is present in a packet side data, the result will be that the
>> parameter sets present in keyframes will be filtered, but the parameter sets
>> ultimately included in the av1C box will not.
>>
>> This is especially important for AV1 as both currently supported encoders
>> don't
>> export the Sequence Header in the codec context extradata, but as packet side
>> data instead.
>>
>> Signed-off-by: James Almer
>> ---
>> libavcodec/av1_metadata_bsf.c | 52 +++
>> 1 file changed, 52 insertions(+)
>>
>> diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
>> index dd0c9b6148..f3161cb286 100644
>> --- a/libavcodec/av1_metadata_bsf.c
>> +++ b/libavcodec/av1_metadata_bsf.c
>> @@ -111,6 +111,54 @@ static int
>> av1_metadata_update_sequence_header(AVBSFContext *bsf,
>> return 0;
>> }
>>
>> +static int av1_metadata_update_side_data(AVBSFContext *bsf, AVPacket *pkt)
>> +{
>> +AV1MetadataContext *ctx = bsf->priv_data;
>> +CodedBitstreamFragment *frag = >access_unit;
>> +AV1RawOBU *obu;
>> +uint8_t *side_data;
>> +int side_data_size;
>> +int err, i;
>> +
>> +side_data = av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
>> +_data_size);
>> +if (!side_data_size)
>> +return 0;
>> +
>> +err = ff_cbs_read(ctx->cbc, frag, side_data, side_data_size);
>> +if (err < 0) {
>> +av_log(bsf, AV_LOG_ERROR, "Failed to read extradata from packet
>> side data.\n");
>> +goto fail;
>
> You can actually return immediately in this whole function: The fragment
> will be reset in the fail part of av1_metadata_filter().
Changed and pushed, thanks.
___
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".
Re: [FFmpeg-devel] [PATCH 1/3] avcodec/av1_metadata: filter parameter sets in packet side data
James Almer:
> Extradata included in packet side data is meant to replace the codec context
> extradata. So when muxing for example to MP4 without this change and if
> extradata is present in a packet side data, the result will be that the
> parameter sets present in keyframes will be filtered, but the parameter sets
> ultimately included in the av1C box will not.
>
> This is especially important for AV1 as both currently supported encoders
> don't
> export the Sequence Header in the codec context extradata, but as packet side
> data instead.
>
> Signed-off-by: James Almer
> ---
> libavcodec/av1_metadata_bsf.c | 52 +++
> 1 file changed, 52 insertions(+)
>
> diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
> index dd0c9b6148..f3161cb286 100644
> --- a/libavcodec/av1_metadata_bsf.c
> +++ b/libavcodec/av1_metadata_bsf.c
> @@ -111,6 +111,54 @@ static int
> av1_metadata_update_sequence_header(AVBSFContext *bsf,
> return 0;
> }
>
> +static int av1_metadata_update_side_data(AVBSFContext *bsf, AVPacket *pkt)
> +{
> +AV1MetadataContext *ctx = bsf->priv_data;
> +CodedBitstreamFragment *frag = >access_unit;
> +AV1RawOBU *obu;
> +uint8_t *side_data;
> +int side_data_size;
> +int err, i;
> +
> +side_data = av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
> +_data_size);
> +if (!side_data_size)
> +return 0;
> +
> +err = ff_cbs_read(ctx->cbc, frag, side_data, side_data_size);
> +if (err < 0) {
> +av_log(bsf, AV_LOG_ERROR, "Failed to read extradata from packet side
> data.\n");
> +goto fail;
You can actually return immediately in this whole function: The fragment
will be reset in the fail part of av1_metadata_filter().
> +}
> +
> +for (i = 0; i < frag->nb_units; i++) {
> +if (frag->units[i].type == AV1_OBU_SEQUENCE_HEADER) {
> +obu = frag->units[i].content;
> +err = av1_metadata_update_sequence_header(bsf,
> >obu.sequence_header);
> +if (err < 0)
> +goto fail;
> +}
> +}
> +
> +err = ff_cbs_write_fragment_data(ctx->cbc, frag);
> +if (err < 0) {
> +av_log(bsf, AV_LOG_ERROR, "Failed to write extradata into packet
> side data.\n");
> +goto fail;
> +}
> +
> +side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
> frag->data_size);
> +if (!side_data) {
> +err = AVERROR(ENOMEM);
> +goto fail;
> +}
> +memcpy(side_data, frag->data, frag->data_size);
> +
> +err = 0;
> +fail:
> +ff_cbs_fragment_reset(ctx->cbc, frag);
> +return err;
> +}
> +
> static int av1_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
> {
> AV1MetadataContext *ctx = bsf->priv_data;
> @@ -122,6 +170,10 @@ static int av1_metadata_filter(AVBSFContext *bsf,
> AVPacket *pkt)
> if (err < 0)
> return err;
>
> +err = av1_metadata_update_side_data(bsf, pkt);
> +if (err < 0)
> +goto fail;
> +
> err = ff_cbs_read_packet(ctx->cbc, frag, pkt);
> if (err < 0) {
> av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
>
___
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".
[FFmpeg-devel] [PATCH 1/3] avcodec/av1_metadata: filter parameter sets in packet side data
Extradata included in packet side data is meant to replace the codec context
extradata. So when muxing for example to MP4 without this change and if
extradata is present in a packet side data, the result will be that the
parameter sets present in keyframes will be filtered, but the parameter sets
ultimately included in the av1C box will not.
This is especially important for AV1 as both currently supported encoders don't
export the Sequence Header in the codec context extradata, but as packet side
data instead.
Signed-off-by: James Almer
---
libavcodec/av1_metadata_bsf.c | 52 +++
1 file changed, 52 insertions(+)
diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
index dd0c9b6148..f3161cb286 100644
--- a/libavcodec/av1_metadata_bsf.c
+++ b/libavcodec/av1_metadata_bsf.c
@@ -111,6 +111,54 @@ static int
av1_metadata_update_sequence_header(AVBSFContext *bsf,
return 0;
}
+static int av1_metadata_update_side_data(AVBSFContext *bsf, AVPacket *pkt)
+{
+AV1MetadataContext *ctx = bsf->priv_data;
+CodedBitstreamFragment *frag = >access_unit;
+AV1RawOBU *obu;
+uint8_t *side_data;
+int side_data_size;
+int err, i;
+
+side_data = av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
+_data_size);
+if (!side_data_size)
+return 0;
+
+err = ff_cbs_read(ctx->cbc, frag, side_data, side_data_size);
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to read extradata from packet side
data.\n");
+goto fail;
+}
+
+for (i = 0; i < frag->nb_units; i++) {
+if (frag->units[i].type == AV1_OBU_SEQUENCE_HEADER) {
+obu = frag->units[i].content;
+err = av1_metadata_update_sequence_header(bsf,
>obu.sequence_header);
+if (err < 0)
+goto fail;
+}
+}
+
+err = ff_cbs_write_fragment_data(ctx->cbc, frag);
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to write extradata into packet side
data.\n");
+goto fail;
+}
+
+side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
frag->data_size);
+if (!side_data) {
+err = AVERROR(ENOMEM);
+goto fail;
+}
+memcpy(side_data, frag->data, frag->data_size);
+
+err = 0;
+fail:
+ff_cbs_fragment_reset(ctx->cbc, frag);
+return err;
+}
+
static int av1_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
{
AV1MetadataContext *ctx = bsf->priv_data;
@@ -122,6 +170,10 @@ static int av1_metadata_filter(AVBSFContext *bsf, AVPacket
*pkt)
if (err < 0)
return err;
+err = av1_metadata_update_side_data(bsf, pkt);
+if (err < 0)
+goto fail;
+
err = ff_cbs_read_packet(ctx->cbc, frag, pkt);
if (err < 0) {
av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
--
2.26.0
___
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".
