| Message ID | 20171027200354.31214-1-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote: > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavcodec/mpegvideo.c | 10 +++++ > libavfilter/vf_codecview.c | 105 +++++++++++++++++++++++++++++++++++++++++++++ > libavutil/frame.h | 4 ++ > 3 files changed, 119 insertions(+) First, thanks for working on this. [...] > diff --git a/libavutil/frame.h b/libavutil/frame.h > index fef558ea2f..8481dc080b 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -141,6 +141,10 @@ enum AVFrameSideDataType { > * metadata key entry "name". > */ > AV_FRAME_DATA_ICC_PROFILE, > + /** > + * Macroblock types exported by some codecs. > + */ > + AV_FRAME_DATA_MACROBLOCK_TYPES, > }; > This makes the internal data of the decoder part of the ABI and API of libavcodec and libavfilter and its undocumented do people prefer to make the internal data part of the ABI, document it and ensure it does not change till the next bump or design a new interface, document it and convert to it? or is there some other option iam missing ? these ABI issues btw are why i said i dont have the time for doing the move. also it seems the patch breaks fate-mov-zombie [...]
Hi, On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer < michael@niedermayer.cc> wrote: > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote: > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > > --- > > libavcodec/mpegvideo.c | 10 +++++ > > libavfilter/vf_codecview.c | 105 ++++++++++++++++++++++++++++++ > +++++++++++++++ > > libavutil/frame.h | 4 ++ > > 3 files changed, 119 insertions(+) > > First, thanks for working on this. > > > [...] > > > diff --git a/libavutil/frame.h b/libavutil/frame.h > > index fef558ea2f..8481dc080b 100644 > > --- a/libavutil/frame.h > > +++ b/libavutil/frame.h > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType { > > * metadata key entry "name". > > */ > > AV_FRAME_DATA_ICC_PROFILE, > > + /** > > + * Macroblock types exported by some codecs. > > + */ > > + AV_FRAME_DATA_MACROBLOCK_TYPES, > > }; > > > > This makes the internal data of the decoder part of the ABI and API of > libavcodec and libavfilter > and its undocumented > > do people prefer to make the internal data part of the ABI, document > it and ensure it does not change till the next bump > [..] > or is there some other option iam missing ? > I noted this on IRC also. A third option is to not document it and consider it codec-specific. (Codec-specific implies "ABI/API unstable" and subject to change - "don't mix versions".) > or design a new interface, document it and convert to it? I think we'd all prefer this, but this requires someone to do the work. Exciting! Ronald
Hi On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote: > Hi, > > On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer < > michael@niedermayer.cc> wrote: > > > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote: > > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > > > --- > > > libavcodec/mpegvideo.c | 10 +++++ > > > libavfilter/vf_codecview.c | 105 ++++++++++++++++++++++++++++++ > > +++++++++++++++ > > > libavutil/frame.h | 4 ++ > > > 3 files changed, 119 insertions(+) > > > > First, thanks for working on this. > > > > > > [...] > > > > > diff --git a/libavutil/frame.h b/libavutil/frame.h > > > index fef558ea2f..8481dc080b 100644 > > > --- a/libavutil/frame.h > > > +++ b/libavutil/frame.h > > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType { > > > * metadata key entry "name". > > > */ > > > AV_FRAME_DATA_ICC_PROFILE, > > > + /** > > > + * Macroblock types exported by some codecs. > > > + */ > > > + AV_FRAME_DATA_MACROBLOCK_TYPES, > > > }; > > > > > > > This makes the internal data of the decoder part of the ABI and API of > > libavcodec and libavfilter > > and its undocumented > > > > do people prefer to make the internal data part of the ABI, document > > it and ensure it does not change till the next bump > > > [..] > > > or is there some other option iam missing ? > > > > I noted this on IRC also. A third option is to not document it and consider > it codec-specific. > > (Codec-specific implies "ABI/API unstable" and subject to change - "don't > mix versions".) If you say the interface is "ABI/API unstable" then we cannot use it from another lib like libavfilter. So this would not work. > > > or design a new interface, document it and convert to it? > > I think we'd all prefer this, but this requires someone to do the work. > Exciting! how generic do we want this to be ? do we have a volunteer to implement this ? because IMO the volunteer doing the work should be the one primarly deciding how generic to do this ... 16x16 MB only, or any size ? first level of subdivision only or multi level ? only H/V divisions at 50% points or any points, what about divisions by non 90° angles? what about non square or non rectangular MBs ? [...]
On 11/2/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > Hi > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote: >> Hi, >> >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer < >> michael@niedermayer.cc> wrote: >> >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote: >> > > Signed-off-by: Paul B Mahol <onemda@gmail.com> >> > > --- >> > > libavcodec/mpegvideo.c | 10 +++++ >> > > libavfilter/vf_codecview.c | 105 ++++++++++++++++++++++++++++++ >> > +++++++++++++++ >> > > libavutil/frame.h | 4 ++ >> > > 3 files changed, 119 insertions(+) >> > >> > First, thanks for working on this. >> > >> > >> > [...] >> > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h >> > > index fef558ea2f..8481dc080b 100644 >> > > --- a/libavutil/frame.h >> > > +++ b/libavutil/frame.h >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType { >> > > * metadata key entry "name". >> > > */ >> > > AV_FRAME_DATA_ICC_PROFILE, >> > > + /** >> > > + * Macroblock types exported by some codecs. >> > > + */ >> > > + AV_FRAME_DATA_MACROBLOCK_TYPES, >> > > }; >> > > >> > >> > This makes the internal data of the decoder part of the ABI and API of >> > libavcodec and libavfilter >> > and its undocumented >> > >> > do people prefer to make the internal data part of the ABI, document >> > it and ensure it does not change till the next bump >> > >> [..] >> >> > or is there some other option iam missing ? >> > >> >> I noted this on IRC also. A third option is to not document it and >> consider >> it codec-specific. >> >> (Codec-specific implies "ABI/API unstable" and subject to change - "don't >> mix versions".) > > If you say the interface is "ABI/API unstable" then we cannot use it > from another lib like libavfilter. So this would not work. I disagree.
Hi, On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol <onemda@gmail.com> wrote: > On 11/2/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > > Hi > > > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote: > >> Hi, > >> > >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer < > >> michael@niedermayer.cc> wrote: > >> > >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote: > >> > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > >> > > --- > >> > > libavcodec/mpegvideo.c | 10 +++++ > >> > > libavfilter/vf_codecview.c | 105 ++++++++++++++++++++++++++++++ > >> > +++++++++++++++ > >> > > libavutil/frame.h | 4 ++ > >> > > 3 files changed, 119 insertions(+) > >> > > >> > First, thanks for working on this. > >> > > >> > > >> > [...] > >> > > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h > >> > > index fef558ea2f..8481dc080b 100644 > >> > > --- a/libavutil/frame.h > >> > > +++ b/libavutil/frame.h > >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType { > >> > > * metadata key entry "name". > >> > > */ > >> > > AV_FRAME_DATA_ICC_PROFILE, > >> > > + /** > >> > > + * Macroblock types exported by some codecs. > >> > > + */ > >> > > + AV_FRAME_DATA_MACROBLOCK_TYPES, > >> > > }; > >> > > > >> > > >> > This makes the internal data of the decoder part of the ABI and API of > >> > libavcodec and libavfilter > >> > and its undocumented > >> > > >> > do people prefer to make the internal data part of the ABI, document > >> > it and ensure it does not change till the next bump > >> > > >> [..] > >> > >> > or is there some other option iam missing ? > >> > > >> > >> I noted this on IRC also. A third option is to not document it and > >> consider > >> it codec-specific. > >> > >> (Codec-specific implies "ABI/API unstable" and subject to change - > "don't > >> mix versions".) > > > > If you say the interface is "ABI/API unstable" then we cannot use it > > from another lib like libavfilter. So this would not work. > > I disagree. I'd like to speak in solutions, not problems. We can simply prepend a version number to the flags and ensure versions match. That makes it easy to break "ABI" (and if versions don't match, simply display an error message and tell the user to not mix versions since this is considered an unstable feature). By no means can we set the current ABI of this feature in stone, we're trying to fix it, not keep it. I appreciate Paul's effort to moving this forward and think his work pushes it in the right direction. Ronald
On Thu, Nov 02, 2017 at 12:04:17PM -0400, Ronald S. Bultje wrote: > Hi, > > On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol <onemda@gmail.com> wrote: > > > On 11/2/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > > > Hi > > > > > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote: > > >> Hi, > > >> > > >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer < > > >> michael@niedermayer.cc> wrote: > > >> > > >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote: > > >> > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > > >> > > --- > > >> > > libavcodec/mpegvideo.c | 10 +++++ > > >> > > libavfilter/vf_codecview.c | 105 ++++++++++++++++++++++++++++++ > > >> > +++++++++++++++ > > >> > > libavutil/frame.h | 4 ++ > > >> > > 3 files changed, 119 insertions(+) > > >> > > > >> > First, thanks for working on this. > > >> > > > >> > > > >> > [...] > > >> > > > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h > > >> > > index fef558ea2f..8481dc080b 100644 > > >> > > --- a/libavutil/frame.h > > >> > > +++ b/libavutil/frame.h > > >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType { > > >> > > * metadata key entry "name". > > >> > > */ > > >> > > AV_FRAME_DATA_ICC_PROFILE, > > >> > > + /** > > >> > > + * Macroblock types exported by some codecs. > > >> > > + */ > > >> > > + AV_FRAME_DATA_MACROBLOCK_TYPES, > > >> > > }; > > >> > > > > >> > > > >> > This makes the internal data of the decoder part of the ABI and API of > > >> > libavcodec and libavfilter > > >> > and its undocumented > > >> > > > >> > do people prefer to make the internal data part of the ABI, document > > >> > it and ensure it does not change till the next bump > > >> > > > >> [..] > > >> > > >> > or is there some other option iam missing ? > > >> > > > >> > > >> I noted this on IRC also. A third option is to not document it and > > >> consider > > >> it codec-specific. > > >> > > >> (Codec-specific implies "ABI/API unstable" and subject to change - > > "don't > > >> mix versions".) > > > > > > If you say the interface is "ABI/API unstable" then we cannot use it > > > from another lib like libavfilter. So this would not work. > > > > I disagree. > > > I'd like to speak in solutions, not problems. We can simply prepend a > version number to the flags and ensure versions match. That makes it easy > to break "ABI" (and if versions don't match, simply display an error > message and tell the user to not mix versions since this is considered an > unstable feature). > > By no means can we set the current ABI of this feature in stone, we're > trying to fix it, not keep it. I appreciate Paul's effort to moving this > forward and think his work pushes it in the right direction. How does this "versioned", "codec specific" API/ABI fit together with the carefully designed strict API used by the same filter to access motion vectors (libavutil/motion_vector.h) ? [...]
On 11/8/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Thu, Nov 02, 2017 at 12:04:17PM -0400, Ronald S. Bultje wrote: >> Hi, >> >> On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol <onemda@gmail.com> wrote: >> >> > On 11/2/17, Michael Niedermayer <michael@niedermayer.cc> wrote: >> > > Hi >> > > >> > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote: >> > >> Hi, >> > >> >> > >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer < >> > >> michael@niedermayer.cc> wrote: >> > >> >> > >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote: >> > >> > > Signed-off-by: Paul B Mahol <onemda@gmail.com> >> > >> > > --- >> > >> > > libavcodec/mpegvideo.c | 10 +++++ >> > >> > > libavfilter/vf_codecview.c | 105 ++++++++++++++++++++++++++++++ >> > >> > +++++++++++++++ >> > >> > > libavutil/frame.h | 4 ++ >> > >> > > 3 files changed, 119 insertions(+) >> > >> > >> > >> > First, thanks for working on this. >> > >> > >> > >> > >> > >> > [...] >> > >> > >> > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h >> > >> > > index fef558ea2f..8481dc080b 100644 >> > >> > > --- a/libavutil/frame.h >> > >> > > +++ b/libavutil/frame.h >> > >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType { >> > >> > > * metadata key entry "name". >> > >> > > */ >> > >> > > AV_FRAME_DATA_ICC_PROFILE, >> > >> > > + /** >> > >> > > + * Macroblock types exported by some codecs. >> > >> > > + */ >> > >> > > + AV_FRAME_DATA_MACROBLOCK_TYPES, >> > >> > > }; >> > >> > > >> > >> > >> > >> > This makes the internal data of the decoder part of the ABI and API >> > >> > of >> > >> > libavcodec and libavfilter >> > >> > and its undocumented >> > >> > >> > >> > do people prefer to make the internal data part of the ABI, >> > >> > document >> > >> > it and ensure it does not change till the next bump >> > >> > >> > >> [..] >> > >> >> > >> > or is there some other option iam missing ? >> > >> > >> > >> >> > >> I noted this on IRC also. A third option is to not document it and >> > >> consider >> > >> it codec-specific. >> > >> >> > >> (Codec-specific implies "ABI/API unstable" and subject to change - >> > "don't >> > >> mix versions".) >> > > >> > > If you say the interface is "ABI/API unstable" then we cannot use it >> > > from another lib like libavfilter. So this would not work. >> > >> > I disagree. >> >> >> I'd like to speak in solutions, not problems. We can simply prepend a >> version number to the flags and ensure versions match. That makes it easy >> to break "ABI" (and if versions don't match, simply display an error >> message and tell the user to not mix versions since this is considered an >> unstable feature). >> >> By no means can we set the current ABI of this feature in stone, we're >> trying to fix it, not keep it. I appreciate Paul's effort to moving this >> forward and think his work pushes it in the right direction. > > How does this "versioned", "codec specific" API/ABI fit together with the > carefully designed strict API used by the same filter to access motion > vectors (libavutil/motion_vector.h) > ? If you do not like it as it is, write specification which I will follow. Otherwise this code gonna be removed from decoder soonTM.
On Wed, Nov 08, 2017 at 12:18:24PM +0100, Paul B Mahol wrote: > On 11/8/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Thu, Nov 02, 2017 at 12:04:17PM -0400, Ronald S. Bultje wrote: > >> Hi, > >> > >> On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol <onemda@gmail.com> wrote: > >> > >> > On 11/2/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > >> > > Hi > >> > > > >> > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote: > >> > >> Hi, > >> > >> > >> > >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer < > >> > >> michael@niedermayer.cc> wrote: > >> > >> > >> > >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote: > >> > >> > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > >> > >> > > --- > >> > >> > > libavcodec/mpegvideo.c | 10 +++++ > >> > >> > > libavfilter/vf_codecview.c | 105 ++++++++++++++++++++++++++++++ > >> > >> > +++++++++++++++ > >> > >> > > libavutil/frame.h | 4 ++ > >> > >> > > 3 files changed, 119 insertions(+) > >> > >> > > >> > >> > First, thanks for working on this. > >> > >> > > >> > >> > > >> > >> > [...] > >> > >> > > >> > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h > >> > >> > > index fef558ea2f..8481dc080b 100644 > >> > >> > > --- a/libavutil/frame.h > >> > >> > > +++ b/libavutil/frame.h > >> > >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType { > >> > >> > > * metadata key entry "name". > >> > >> > > */ > >> > >> > > AV_FRAME_DATA_ICC_PROFILE, > >> > >> > > + /** > >> > >> > > + * Macroblock types exported by some codecs. > >> > >> > > + */ > >> > >> > > + AV_FRAME_DATA_MACROBLOCK_TYPES, > >> > >> > > }; > >> > >> > > > >> > >> > > >> > >> > This makes the internal data of the decoder part of the ABI and API > >> > >> > of > >> > >> > libavcodec and libavfilter > >> > >> > and its undocumented > >> > >> > > >> > >> > do people prefer to make the internal data part of the ABI, > >> > >> > document > >> > >> > it and ensure it does not change till the next bump > >> > >> > > >> > >> [..] > >> > >> > >> > >> > or is there some other option iam missing ? > >> > >> > > >> > >> > >> > >> I noted this on IRC also. A third option is to not document it and > >> > >> consider > >> > >> it codec-specific. > >> > >> > >> > >> (Codec-specific implies "ABI/API unstable" and subject to change - > >> > "don't > >> > >> mix versions".) > >> > > > >> > > If you say the interface is "ABI/API unstable" then we cannot use it > >> > > from another lib like libavfilter. So this would not work. > >> > > >> > I disagree. > >> > >> > >> I'd like to speak in solutions, not problems. We can simply prepend a > >> version number to the flags and ensure versions match. That makes it easy > >> to break "ABI" (and if versions don't match, simply display an error > >> message and tell the user to not mix versions since this is considered an > >> unstable feature). > >> > >> By no means can we set the current ABI of this feature in stone, we're > >> trying to fix it, not keep it. I appreciate Paul's effort to moving this > >> forward and think his work pushes it in the right direction. > > > > How does this "versioned", "codec specific" API/ABI fit together with the > > carefully designed strict API used by the same filter to access motion > > vectors (libavutil/motion_vector.h) > > ? > > If you do not like it as it is, write specification which I will follow. I can. Is there some preferrances that people have ? this can be done in a wide range from simple to quite complex. from a simple bitmask in a byte or int array to a tree structure describing subdivisions or a list of structs similar to libavutil/motion_vector.h. The simple one will not be able to represent everything > Otherwise this code gonna be removed from decoder soonTM. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 11/8/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Nov 08, 2017 at 12:18:24PM +0100, Paul B Mahol wrote: >> On 11/8/17, Michael Niedermayer <michael@niedermayer.cc> wrote: >> > On Thu, Nov 02, 2017 at 12:04:17PM -0400, Ronald S. Bultje wrote: >> >> Hi, >> >> >> >> On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol <onemda@gmail.com> wrote: >> >> >> >> > On 11/2/17, Michael Niedermayer <michael@niedermayer.cc> wrote: >> >> > > Hi >> >> > > >> >> > > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote: >> >> > >> Hi, >> >> > >> >> >> > >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer < >> >> > >> michael@niedermayer.cc> wrote: >> >> > >> >> >> > >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote: >> >> > >> > > Signed-off-by: Paul B Mahol <onemda@gmail.com> >> >> > >> > > --- >> >> > >> > > libavcodec/mpegvideo.c | 10 +++++ >> >> > >> > > libavfilter/vf_codecview.c | 105 >> >> > >> > > ++++++++++++++++++++++++++++++ >> >> > >> > +++++++++++++++ >> >> > >> > > libavutil/frame.h | 4 ++ >> >> > >> > > 3 files changed, 119 insertions(+) >> >> > >> > >> >> > >> > First, thanks for working on this. >> >> > >> > >> >> > >> > >> >> > >> > [...] >> >> > >> > >> >> > >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h >> >> > >> > > index fef558ea2f..8481dc080b 100644 >> >> > >> > > --- a/libavutil/frame.h >> >> > >> > > +++ b/libavutil/frame.h >> >> > >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType { >> >> > >> > > * metadata key entry "name". >> >> > >> > > */ >> >> > >> > > AV_FRAME_DATA_ICC_PROFILE, >> >> > >> > > + /** >> >> > >> > > + * Macroblock types exported by some codecs. >> >> > >> > > + */ >> >> > >> > > + AV_FRAME_DATA_MACROBLOCK_TYPES, >> >> > >> > > }; >> >> > >> > > >> >> > >> > >> >> > >> > This makes the internal data of the decoder part of the ABI and >> >> > >> > API >> >> > >> > of >> >> > >> > libavcodec and libavfilter >> >> > >> > and its undocumented >> >> > >> > >> >> > >> > do people prefer to make the internal data part of the ABI, >> >> > >> > document >> >> > >> > it and ensure it does not change till the next bump >> >> > >> > >> >> > >> [..] >> >> > >> >> >> > >> > or is there some other option iam missing ? >> >> > >> > >> >> > >> >> >> > >> I noted this on IRC also. A third option is to not document it >> >> > >> and >> >> > >> consider >> >> > >> it codec-specific. >> >> > >> >> >> > >> (Codec-specific implies "ABI/API unstable" and subject to change >> >> > >> - >> >> > "don't >> >> > >> mix versions".) >> >> > > >> >> > > If you say the interface is "ABI/API unstable" then we cannot use >> >> > > it >> >> > > from another lib like libavfilter. So this would not work. >> >> > >> >> > I disagree. >> >> >> >> >> >> I'd like to speak in solutions, not problems. We can simply prepend a >> >> version number to the flags and ensure versions match. That makes it >> >> easy >> >> to break "ABI" (and if versions don't match, simply display an error >> >> message and tell the user to not mix versions since this is considered >> >> an >> >> unstable feature). >> >> >> >> By no means can we set the current ABI of this feature in stone, we're >> >> trying to fix it, not keep it. I appreciate Paul's effort to moving >> >> this >> >> forward and think his work pushes it in the right direction. >> > >> > How does this "versioned", "codec specific" API/ABI fit together with >> > the >> > carefully designed strict API used by the same filter to access motion >> > vectors (libavutil/motion_vector.h) >> > ? >> >> If you do not like it as it is, write specification which I will follow. > > I can. Is there some preferrances that people have ? > this can be done in a wide range from simple to quite complex. > from a simple bitmask in a byte or int array to a tree structure > describing subdivisions or a list of structs similar to > libavutil/motion_vector.h. > The simple one will not be able to represent everything Just one that gonna be used by single codec. More complex one can be added when needed.
diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c index 2f5793b9a4..ce6108d094 100644 --- a/libavcodec/mpegvideo.c +++ b/libavcodec/mpegvideo.c @@ -1603,6 +1603,16 @@ void ff_print_debug_info2(AVCodecContext *avctx, AVFrame *pict, uint8_t *mbskip_ } } + if (mb_height && mb_width) { + AVFrameSideData *sd; + + av_log(avctx, AV_LOG_DEBUG, "Adding %d MB types info to frame %d\n", mb_width * mb_height, avctx->frame_number); + sd = av_frame_new_side_data(pict, AV_FRAME_DATA_MACROBLOCK_TYPES, mb_width * mb_height * sizeof(uint32_t)); + if (!sd) + return; + memcpy(sd->data, mbtype_table, mb_width * mb_height * sizeof(uint32_t)); + } + #if FF_API_DEBUG_MV if ((avctx->debug & (FF_DEBUG_VIS_QP | FF_DEBUG_VIS_MB_TYPE)) || (avctx->debug_mv)) { diff --git a/libavfilter/vf_codecview.c b/libavfilter/vf_codecview.c index 331bfba777..40fb8dfb7a 100644 --- a/libavfilter/vf_codecview.c +++ b/libavfilter/vf_codecview.c @@ -29,6 +29,7 @@ * TODO: segmentation */ +#include "libavutil/avassert.h" #include "libavutil/imgutils.h" #include "libavutil/motion_vector.h" #include "libavutil/opt.h" @@ -44,9 +45,23 @@ #define FRAME_TYPE_P (1<<1) #define FRAME_TYPE_B (1<<2) +#define IS_PCM(a) ((a) & (1 << 2)) +#define IS_INTRA(a) ((a) & 7) +#define IS_ACPRED(a) ((a) & (1 << 9)) +#define IS_INTRA16x16(a) ((a) & (1 << 1)) +#define IS_INTRA4x4(a) ((a) & (1 << 0)) +#define IS_DIRECT(a) ((a) & (1 << 8)) +#define IS_SKIP(a) ((a) & (1 << 11)) +#define IS_GMC(a) ((a) & (1 << 10)) +#define USES_LIST(a, list) ((a) & (((1 << 12) | (1 << 13)) << (2 * (list)))) +#define IS_8X8(a) ((a) & ((1 << 6))) +#define IS_16X8(a) ((a) & ((1 << 4))) +#define IS_8X16(a) ((a) & ((1 << 5))) + typedef struct CodecViewContext { const AVClass *class; unsigned mv; + unsigned mbtypes; unsigned frame_type; unsigned mv_type; int hsub, vsub; @@ -72,6 +87,7 @@ static const AVOption codecview_options[] = { CONST("if", "I-frames", FRAME_TYPE_I, "frame_type"), CONST("pf", "P-frames", FRAME_TYPE_P, "frame_type"), CONST("bf", "B-frames", FRAME_TYPE_B, "frame_type"), + { "mb", "visualize macroblock types", OFFSET(mbtypes), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS, }, { NULL } }; @@ -277,6 +293,95 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) } } + if (s->mbtypes) { + AVFrameSideData *sd = av_frame_get_side_data(frame, AV_FRAME_DATA_MACROBLOCK_TYPES); + if (sd) { + int mb_height = (frame->height + 31) / 32 * 2; + int mb_width = (frame->width + 15) / 16; + int block_height = 16 >> 1; + int mb_stride = mb_width + 1; + int mb_y, mb_x; + uint32_t *mbtype_table = (uint32_t *)sd->data; + + for (mb_y = 0; mb_y < mb_height; mb_y++) { + for (mb_x = 0; mb_x < mb_width; mb_x++) { + const int mb_index = mb_x + mb_y * mb_stride; + /*{ + uint64_t c = (qscale_table[mb_index] * 128 / 31) * + 0x0101010101010101ULL; + int y; + for (y = 0; y < block_height; y++) { + *(uint64_t *)(frame->data[1] + 8 * mb_x + + (block_height * mb_y + y) * + frame->linesize[1]) = c; + *(uint64_t *)(frame->data[2] + 8 * mb_x + + (block_height * mb_y + y) * + frame->linesize[2]) = c; + } + } */ + { + int mb_type = mbtype_table[mb_index]; + uint64_t u,v; + int y; + +#define COLOR(theta, r) \ + u = (int)(128 + r * cos(theta * M_PI / 180)); \ + v = (int)(128 + r * sin(theta * M_PI / 180)); + + u = v = 128; + if (IS_PCM(mb_type)) { + COLOR(120, 48) + } else if ((IS_INTRA(mb_type) && IS_ACPRED(mb_type)) || + IS_INTRA16x16(mb_type)) { + COLOR(30, 48) + } else if (IS_INTRA4x4(mb_type)) { + COLOR(90, 48) + } else if (IS_DIRECT(mb_type) && IS_SKIP(mb_type)) { + COLOR(120, 48) + } else if (IS_DIRECT(mb_type)) { + COLOR(150, 48) + } else if (IS_GMC(mb_type) && IS_SKIP(mb_type)) { + COLOR(170, 48) + } else if (IS_GMC(mb_type)) { + COLOR(190, 48) + } else if (IS_SKIP(mb_type)) { + COLOR(180, 48) + } else if (!USES_LIST(mb_type, 1)) { + COLOR(240, 48) + } else if (!USES_LIST(mb_type, 0)) { + COLOR(0, 48) + } else { + av_assert2(USES_LIST(mb_type, 0) && USES_LIST(mb_type, 1)); + COLOR(300,48) + } + + u *= 0x0101010101010101ULL; + v *= 0x0101010101010101ULL; + for (y = 0; y < block_height; y++) { + *(uint64_t *)(frame->data[1] + 8 * mb_x + + (block_height * mb_y + y) * frame->linesize[1]) = u; + *(uint64_t *)(frame->data[2] + 8 * mb_x + + (block_height * mb_y + y) * frame->linesize[2]) = v; + } + + // segmentation + if (IS_8X8(mb_type) || IS_16X8(mb_type)) { + *(uint64_t *)(frame->data[0] + 16 * mb_x + 0 + + (16 * mb_y + 8) * frame->linesize[0]) ^= 0x8080808080808080ULL; + *(uint64_t *)(frame->data[0] + 16 * mb_x + 8 + + (16 * mb_y + 8) * frame->linesize[0]) ^= 0x8080808080808080ULL; + } + if (IS_8X8(mb_type) || IS_8X16(mb_type)) { + for (y = 0; y < 16; y++) + frame->data[0][16 * mb_x + 8 + (16 * mb_y + y) * + frame->linesize[0]] ^= 0x80; + } + } + } + } + } + } + return ff_filter_frame(outlink, frame); } diff --git a/libavutil/frame.h b/libavutil/frame.h index fef558ea2f..8481dc080b 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -141,6 +141,10 @@ enum AVFrameSideDataType { * metadata key entry "name". */ AV_FRAME_DATA_ICC_PROFILE, + /** + * Macroblock types exported by some codecs. + */ + AV_FRAME_DATA_MACROBLOCK_TYPES, }; enum AVActiveFormatDescription {
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavcodec/mpegvideo.c | 10 +++++ libavfilter/vf_codecview.c | 105 +++++++++++++++++++++++++++++++++++++++++++++ libavutil/frame.h | 4 ++ 3 files changed, 119 insertions(+)