[FFmpeg-devel] [PATCH] avcodec: export motion vectors in frame side data on demand
Clément Bœsch
u at pkh.me
Sun Aug 17 20:08:35 CEST 2014
On Thu, Aug 14, 2014 at 01:19:10PM +0200, Stefano Sabatini wrote:
> On date Monday 2014-08-11 15:22:59 +0200, Clément Bœsch encoded:
> > From: Clément Bœsch <clement at stupeflix.com>
> >
> > The reasoning behind this addition is that various third party
> > applications are interested in getting some motion information out of a
> > video "for free" when it is available.
> >
> > It was considered to export other information as well (such as the intra
> > information about the block, or the quantization) but the structure
> > might have ended up into a half full-generic, half full of codec
> > specific cruft. If more information is necessary, it should either be
> > added in the "flags" field of the AVExportedMV structure, or in another
> > side-data.
> >
> > This commit also includes an example exporting them in a CSV stream.
> > ---
> > TODO: avcodec version bump & APIChanges entry
> > ---
> > .gitignore | 1 +
> > configure | 2 +
> > doc/Makefile | 1 +
> > doc/codecs.texi | 3 +
> > doc/examples/Makefile | 1 +
> > doc/examples/extract_mvs.c | 185 +++++++++++++++++++++++++++++++++++++++++++++
> > libavcodec/avcodec.h | 1 +
> > libavcodec/mpegvideo.c | 102 ++++++++++++++++++++++++-
> > libavcodec/options_table.h | 1 +
> > libavutil/frame.h | 6 ++
>
> > libavutil/mvinfo.h | 49 ++++++++++++
>
> You probably need to add this to the list of public headers in
> libavcodec/Makefile.
>
I was sure to have added it there... fixed in libavutil/Makefile
[...]
> Alternatively, hack demuxing_decoding.c since much code is shared
> (this has pluses - less code duplication, smaller maintainance cost, and
> cons - more complexity for the user).
>
Mmh sorry I really prefer a dedicated example for this.
> Also, do you think this could be exposed by ffprobe?
>
That's a good point; I guess we could. I just made sure for now to
recognize the side data type as "Motion vectors" in
lavu/frame.c:av_frame_side_data_name(). Patch welcome I suppose.
[...]
> > + /* width * height * directions * 4MB (4MB for IS_8x8) */
>
> this comment together with the following "2 * 4" is confusing
> (especially for a naive reader - as me)
>
Explicited with:
/* size is width * height * 2 * 4 where 2 is for directions and 4 is
for the maximum number of MB (4 MB in case of IS_8x8) */
Hopefully that's clear enough now.
> > + AVExportedMV *mvs = av_malloc_array(mb_width * mb_height, 2 * 4 * sizeof(AVExportedMV));
>
> > + if (!mvs)
> > + return;
> > +
> > + for (mb_y = 0; mb_y < mb_height; mb_y++) {
> > + for (mb_x = 0; mb_x < mb_width; mb_x++) {
> > + int i, direction, mb_type = mbtype_table[mb_x + mb_y * mb_stride];
> > + for (direction = 0; direction < 2; direction++) {
> > + if (!USES_LIST(mb_type, direction))
> > + continue;
> > + if (IS_8X8(mb_type)) {
> > + for (i = 0; i < 4; i++) {
> > + int sx = mb_x * 16 + 4 + 8 * (i & 1);
> > + int sy = mb_y * 16 + 4 + 8 * (i >> 1);
> > + int xy = (mb_x * 2 + (i & 1) +
> > + (mb_y * 2 + (i >> 1)) * mv_stride) << (mv_sample_log2 - 1);
> > + int mx = (motion_val[direction][xy][0] >> shift) + sx;
> > + int my = (motion_val[direction][xy][1] >> shift) + sy;
> > + mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, mx, my, direction);
> > + }
> > + } else if (IS_16X8(mb_type)) {
> > + for (i = 0; i < 2; i++) {
> > + int sx = mb_x * 16 + 8;
> > + int sy = mb_y * 16 + 4 + 8 * i;
> > + int xy = (mb_x * 2 + (mb_y * 2 + i) * mv_stride) << (mv_sample_log2 - 1);
> > + int mx = (motion_val[direction][xy][0] >> shift);
> > + int my = (motion_val[direction][xy][1] >> shift);
> > +
> > + if (IS_INTERLACED(mb_type))
> > + my *= 2;
> > +
> > + mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, mx + sx, my + sy, direction);
> > + }
> > + } else if (IS_8X16(mb_type)) {
> > + for (i = 0; i < 2; i++) {
> > + int sx = mb_x * 16 + 4 + 8 * i;
> > + int sy = mb_y * 16 + 8;
> > + int xy = (mb_x * 2 + i + mb_y * 2 * mv_stride) << (mv_sample_log2 - 1);
> > + int mx = motion_val[direction][xy][0] >> shift;
> > + int my = motion_val[direction][xy][1] >> shift;
> > +
> > + if (IS_INTERLACED(mb_type))
> > + my *= 2;
> > +
> > + mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, mx + sx, my + sy, direction);
> > + }
> > + } else {
> > + int sx = mb_x * 16 + 8;
> > + int sy = mb_y * 16 + 8;
> > + int xy = (mb_x + mb_y * mv_stride) << mv_sample_log2;
> > + int mx = (motion_val[direction][xy][0]>>shift) + sx;
> > + int my = (motion_val[direction][xy][1]>>shift) + sy;
> > + mbcount += add_mb(mvs + mbcount, mb_type, sx, sy, mx, my, direction);
> > + }
> > + }
> > + }
> > + }
>
> Uhm, duplicated non-trivial code, probably you can create a dedicated
> routine to factorize this with the code below in the function.
>
Actually, the code below is supposed to be dropped at some point (when
every information is exported we can create a video filter for this), and
it was too much hassle to factorize it for something we would probably
revert later. See the TODO below:
/* TODO: export all the following to make them accessible for users (and filters) */
[...]
> > + /**
> > + * Motion vectors exported by some codecs (on demand through
> > + * -flags2 export_mvs).
>
> Nit, since this is library documentation:
>
> * Motion vectors exported by some codecs (on demand through the
> * export_mvs flag set in the libavcodec AVCodecContext flags2
> * option).
>
Changed.
> > + * The data is the AVExportedMV struct defined in libavutil/mvinfo.h.
> > + */
> > + AV_FRAME_DATA_MV_INFO,
>
> Nit+: I'd prefer to color it as AV_FRAME_DATA_MV or
> AV_FRAME_DATA_MOTION_VECTOR, since "INFO" is really generic (and is
> never used in the other enum named constants).
>
Agreed, renamed to AV_FRAME_DATA_MOTION_VECTOR
> > };
> >
> > enum AVActiveFormatDescription {
> > diff --git a/libavutil/mvinfo.h b/libavutil/mvinfo.h
> > new file mode 100644
> > index 0000000..735f1b9
> > --- /dev/null
> > +++ b/libavutil/mvinfo.h
> > @@ -0,0 +1,49 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
>
> > +#ifndef AVUTIL_MVINFO_H
> > +#define AVUTIL_MVINFO_H
> > +
> > +#include <stdint.h>
> > +
> > +typedef struct AVExportedMV {
> > + /**
> > + * Where the current comes from; negative value for past, positive value future.
> > + * XXX: set exact relative ref frame reference instead of a +/- 1 "direction".
> > + */
> > + int32_t source;
> > + /**
> > + * Width and height of the block.
> > + */
> > + uint8_t w, h;
> > + /**
> > + * Absolute source position.
> > + */
> > + uint16_t src_x, src_y;
> > + /**
> > + * Absolute destination position.
> > + */
> > + uint16_t dst_x, dst_y;
> > + /**
> > + * Extra flag information.
> > + * Currently unused.
> > + */
> > + uint64_t flags;
> > +} AVExportedMV;
> > +
> > +#endif /* AVUTIL_MVINFO_H */
>
> Nit++: AVMVInfo or simply AVMotionVector ("Exported" and "Info" are too
> generic).
I like Exported because it was meant to be with side-data. Someone
complained about SD (SideData / SmallDefinition), so I picked that word.
Anyway, I renamed it to AVMotionVector, and the header is now
libavutil/motion_vector.h
I will push soon the attached patch.
--
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avcodec-export-motion-vectors-in-frame-side-data-on-.patch
Type: text/x-diff
Size: 23147 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140817/9ffa3717/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140817/9ffa3717/attachment.asc>
More information about the ffmpeg-devel
mailing list