On Fri, 2 Mar 2018 12:16:37 +0100 wm4 <nfx...@googlemail.com> wrote:
> This adds a way for an API user to transfer QP data and metadata without > having to keep the reference to AVFrame, and without having to > explicitly care about QP APIs. It might also provide a way to finally > remove the deprecated QP related fields. In the end, the QP table should > be handled in a very similar way to e.g. AV_FRAME_DATA_MOTION_VECTORS. > > There are two side data types, because I didn't care about having to > repack the QP data so the table and the metadata are in a single > AVBufferRef. Otherwise it would have either required a copy on decoding > (extra slowdown for something as obscure as the QP data), or would have > required making intrusive changes to the codecs which support export of > this data. > > The new side data types are added under deprecation guards, because I > don't intend to change the status of the QP export as being deprecated > (as it was before this patch too). > --- > libavutil/frame.c | 59 > ++++++++++++++++++++++++++++++++++---- > libavutil/frame.h | 17 +++++++++++ > tests/ref/fate/exif-image-embedded | 6 ++++ > tests/ref/fate/exif-image-jpg | 34 +++++++++++++--------- > 4 files changed, 96 insertions(+), 20 deletions(-) > > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 0db2a2d57b..ea13cd3ed6 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -46,8 +46,17 @@ MAKE_ACCESSORS(AVFrame, frame, enum AVColorRange, > color_range) > av_get_channel_layout_nb_channels((frame)->channel_layout)) > > #if FF_API_FRAME_QP > +struct qp_properties { > + int stride; > + int type; > +}; > + > int av_frame_set_qp_table(AVFrame *f, AVBufferRef *buf, int stride, int > qp_type) > { > + struct qp_properties *p; > + AVFrameSideData *sd; > + AVBufferRef *ref; > + > FF_DISABLE_DEPRECATION_WARNINGS > av_buffer_unref(&f->qp_table_buf); > > @@ -57,20 +66,56 @@ FF_DISABLE_DEPRECATION_WARNINGS > f->qscale_type = qp_type; > FF_ENABLE_DEPRECATION_WARNINGS > > + av_frame_remove_side_data(f, AV_FRAME_DATA_QP_TABLE_PROPERTIES); > + av_frame_remove_side_data(f, AV_FRAME_DATA_QP_TABLE_DATA); > + > + ref = av_buffer_ref(buf); > + if (!av_frame_new_side_data_from_buf(f, AV_FRAME_DATA_QP_TABLE_DATA, > ref)) { > + av_buffer_unref(&ref); > + return AVERROR(ENOMEM); > + } > + > + sd = av_frame_new_side_data(f, AV_FRAME_DATA_QP_TABLE_PROPERTIES, > + sizeof(struct qp_properties)); > + if (!sd) > + return AVERROR(ENOMEM); > + > + p = (struct qp_properties *)sd->data; > + p->stride = stride; > + p->type = qp_type; > + > return 0; > } > > int8_t *av_frame_get_qp_table(AVFrame *f, int *stride, int *type) > { > -FF_DISABLE_DEPRECATION_WARNINGS > - *stride = f->qstride; > - *type = f->qscale_type; > + AVBufferRef *buf = NULL; > > - if (!f->qp_table_buf) > - return NULL; > + *stride = 0; > + *type = 0; > > - return f->qp_table_buf->data; > +FF_DISABLE_DEPRECATION_WARNINGS > + if (f->qp_table_buf) { > + *stride = f->qstride; > + *type = f->qscale_type; > + buf = f->qp_table_buf; > FF_ENABLE_DEPRECATION_WARNINGS > + } else { > + AVFrameSideData *sd; > + struct qp_properties *p; > + sd = av_frame_get_side_data(f, AV_FRAME_DATA_QP_TABLE_PROPERTIES); > + if (!sd) > + return NULL; > + p = (struct qp_properties *)sd->data; > + sd = av_frame_get_side_data(f, AV_FRAME_DATA_QP_TABLE_DATA); > + if (!sd) > + return NULL; > + *stride = p->stride; > + *type = p->type; > + buf = sd->buf; > + } > + > + return buf ? buf->data : NULL; > } > #endif > > @@ -787,6 +832,8 @@ const char *av_frame_side_data_name(enum > AVFrameSideDataType type) > case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL: return "Content light > level metadata"; > case AV_FRAME_DATA_GOP_TIMECODE: return "GOP timecode"; > case AV_FRAME_DATA_ICC_PROFILE: return "ICC profile"; > + case AV_FRAME_DATA_QP_TABLE_PROPERTIES: return "QP table > properties"; > + case AV_FRAME_DATA_QP_TABLE_DATA: return "QP table data"; > } > return NULL; > } > diff --git a/libavutil/frame.h b/libavutil/frame.h > index ddbac3156d..9d57d6ce66 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -141,6 +141,23 @@ enum AVFrameSideDataType { > * metadata key entry "name". > */ > AV_FRAME_DATA_ICC_PROFILE, > + > +#if FF_API_FRAME_QP > + /** > + * Implementation-specific description of the format of > AV_FRAME_QP_TABLE_DATA. > + * The contents of this side data are undocumented and internal; use > + * av_frame_set_qp_table() and av_frame_get_qp_table() to access this in > a > + * meaningful way instead. > + */ > + AV_FRAME_DATA_QP_TABLE_PROPERTIES, > + > + /** > + * Raw QP table data. Its format is described by > + * AV_FRAME_DATA_QP_TABLE_PROPERTIES. Use av_frame_set_qp_table() and > + * av_frame_get_qp_table() to access this instead. > + */ > + AV_FRAME_DATA_QP_TABLE_DATA, > +#endif > }; > > enum AVActiveFormatDescription { > diff --git a/tests/ref/fate/exif-image-embedded > b/tests/ref/fate/exif-image-embedded > index 306ae0854b..0b640767a8 100644 > --- a/tests/ref/fate/exif-image-embedded > +++ b/tests/ref/fate/exif-image-embedded > @@ -29,6 +29,12 @@ color_transfer=unknown > chroma_location=center > TAG:UserComment=AppleMark > > +[SIDE_DATA] > +side_data_type=QP table data > +[/SIDE_DATA] > +[SIDE_DATA] > +side_data_type=QP table properties > +[/SIDE_DATA] > [/FRAME] > [FRAME] > media_type=audio > diff --git a/tests/ref/fate/exif-image-jpg b/tests/ref/fate/exif-image-jpg > index b266501191..dec2b78737 100644 > --- a/tests/ref/fate/exif-image-jpg > +++ b/tests/ref/fate/exif-image-jpg > @@ -27,30 +27,30 @@ color_space=bt470bg > color_primaries=unknown > color_transfer=unknown > chroma_location=center > -TAG:ImageDescription= > +TAG:ImageDescription= > TAG:Make=Canon > TAG:Model=Canon PowerShot SX200 IS > TAG:Orientation= 1 > -TAG:XResolution= 180:1 > -TAG:YResolution= 180:1 > +TAG:XResolution= 180:1 > +TAG:YResolution= 180:1 > TAG:ResolutionUnit= 2 > TAG:DateTime=2013:07:18 13:12:03 > TAG:YCbCrPositioning= 2 > -TAG:ExposureTime= 1:1250 > -TAG:FNumber= 40:10 > +TAG:ExposureTime= 1:1250 > +TAG:FNumber= 40:10 > TAG:ISOSpeedRatings= 160 > TAG:ExifVersion= 48, 50, 50, 49 > TAG:DateTimeOriginal=2013:07:18 13:12:03 > TAG:DateTimeDigitized=2013:07:18 13:12:03 > TAG:ComponentsConfiguration= 1, 2, 3, 0 > -TAG:CompressedBitsPerPixel= 3:1 > -TAG:ShutterSpeedValue= 329:32 > -TAG:ApertureValue= 128:32 > -TAG:ExposureBiasValue= 0:3 > -TAG:MaxApertureValue= 113:32 > +TAG:CompressedBitsPerPixel= 3:1 > +TAG:ShutterSpeedValue= 329:32 > +TAG:ApertureValue= 128:32 > +TAG:ExposureBiasValue= 0:3 > +TAG:MaxApertureValue= 113:32 > TAG:MeteringMode= 5 > TAG:Flash= 16 > -TAG:FocalLength= 5000:1000 > +TAG:FocalLength= 5000:1000 > TAG:MakerNote= > 25, 0, 1, 0, 3, 0, 48, 0, 0, 0, 28, 4, 0, 0, 2, > 0 > 3, 0, 4, 0, 0, 0, 124, 4, 0, 0, 3, 0, 3, 0, 4, > 0 > @@ -219,14 +219,20 @@ TAG:GPSLatitudeRef=R98 > TAG:GPSLatitude= 48, 49, 48, 48 > TAG:0x1001= 4000 > TAG:0x1002= 2248 > -TAG:FocalPlaneXResolution=4000000:244 > -TAG:FocalPlaneYResolution=2248000:183 > +TAG:FocalPlaneXResolution=4000000:244 > +TAG:FocalPlaneYResolution=2248000:183 > TAG:FocalPlaneResolutionUnit= 2 > TAG:SensingMethod= 2 > TAG:FileSource= 3 > TAG:CustomRendered= 0 > TAG:ExposureMode= 0 > TAG:WhiteBalance= 0 > -TAG:DigitalZoomRatio= 4000:4000 > +TAG:DigitalZoomRatio= 4000:4000 > TAG:SceneCaptureType= 0 > +[SIDE_DATA] > +side_data_type=QP table data > +[/SIDE_DATA] > +[SIDE_DATA] > +side_data_type=QP table properties > +[/SIDE_DATA] > [/FRAME] It has been pointed out that due to AVBuffer copy-on-write, putting pointers into side data can't work anyway. So unless someone has a strong opinion that I should make the (probably little) effort to change the decoders to allocate the qp table in a way that you can put side data including metadata into a single AVBufferRef without copying, I'll leave it at 2 side data types. The added whitespace is mysterious, but I assume it doesn't break FATE for others. I will push these patches on the weekend unless someone has a different opinion. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel