[MPlayer-dev-eng] [PATCH] Add IMGFMT_444P and IMGFMT_422P to vo_md5sum.c
Giorgio
mywing81 at gmail.com
Tue Jun 8 17:18:42 CEST 2010
2010/6/6 Reimar Döffinger <Reimar.Doeffinger at gmx.de>:
> On Sun, Jun 06, 2010 at 08:02:48PM +0200, Giorgio wrote:
>> Reading the code I got the impression that mpi->width and mpi->w have
>> always the same value (when an image is created in new_mp_image(),
>> these values are initialized like this: mpi->width=mpi->w=w;). The
>> same happens with mpi->height and mpi->h. So I used mpi->width and
>> mpi->height because I think it's more clear. Is this correct?
>
> Oh, I know people got used to MPlayer having no useful documentation,
> it still is a good idea to look for it...
> int width,height; // stored dimensions
> int x,y,w,h; // visible dimensions
> So no, it is plain and simple wrong.
Yes, I looked there, but I didn't understand completely what "stored
dimensions" and "visible dimensions" meant in that context.
So I tried to understand it reading the code. Let's suppose we use
queens-444.ogv [1] (theora, 4:4:4 chroma subsampling) and run mplayer
with the command line:
mplayer -vo md5sum -v -quiet -frames 3 -vc theora queens-444.ogv
Now I will analyze what happens starting from the decoder. This is
output that I get (the last part obviously, and some messages are
slightly modified for better clarity):
1:==========================================================================
2:Forced video codec: theora
3:Opening video decoder: [theora] Theora/VP3
4:theora: Theora video init ok!
5:theora: Frame 320x240, Picture 320x240, Offset [0,0]
6:VDec: vo config request - 320 x 240 (preferred colorspace: Planar 444P)
7:VDec: trying filter chain: vo
8:VDec: vo_debug: query(Planar YV12,0x32315659) returned 0x403 (i=0)
9:VDec: vo_debug: codec query_format(Planar YV12,0x32315659) returned FALSE
10:VDec: vo_debug: query(Planar 422P,0x50323234) returned 0x3 (i=1)
11:VDec: vo_debug: codec query_format(Planar 422P,0x50323234) returned FALSE
12:VDec: vo_debug: query(Planar 444P,0x50343434) returned 0x3 (i=2)
13:VDec: vo_debug: codec query_format(Planar 444P,0x50343434) returned TRUE
14:VDec: using Planar 444P as output colorspace (n: 2)
15:Movie-Aspect is 1.33:1 - prescaling to correct movie aspect.
16:VDec: VO Config (320x240->320x240,flags=0,'MPlayer',0x50343434)
17:VO: [md5sum] 320x240 => 320x240 Planar 444P
18:VO: Description: md5sum of each frame
19:VO: Author: Ivo van Poorten (ivop at euronet.nl)
20:Selected video codec: [theora] vfm: theora (Theora (free, reworked VP3))
21:==========================================================================
22:Audio: no sound
23:Freeing 0 unused audio chunks.
24:Starting playback...
25:
26:theora: mpcodecs_get_image()
27:vf: vf_get_image(): [vo] w=320 h=240, vf->w=320 vf->h=240
28:vf: imgtype=export
29:vf: mp_image_setfmt()
30:*** [vo] Exporting mp_image_t, 320x240x24bpp YUV planar, 230400 bytes
31:vo_md5sum: draw_image() w=320 h=240
32:
33:theora: mpcodecs_get_image()
34:vf: vf_get_image(): [vo] w=320 h=240, vf->w=320 vf->h=240
35:vo_md5sum: draw_image() w=320 h=240
36:
37:theora: mpcodecs_get_image()
38:vf: vf_get_image(): [vo] w=320 h=240, vf->w=320 vf->h=240
39:vo_md5sum: draw_image() w=320 h=240
40:EOF code: 1
41:
42:Uninit video: theora
43:vo: x11 uninit called but X11 not initialized..
44:
45:Exiting... (End of file)
step1) The first thing to do is opening the video decoder and calling
vd_theora.c::init() (lines 3-5). Everything seems to go well.
step2) init() calls mpcodecs_config_vo () with preferred colorspace
Planar 444P (line 6). In the filter chain we have only the vo wrapper
(line7). Now, the colorspace matching algorithm tries to find the best
colorspace that works for both the decoder and the filter chain/vo.
After 3 tries it chooses Planar 444P (lines 8-14). Then it takes into
account the aspect ratio (line 15) and finally configures the filter
chain (line 16). But since we only have the vo wrapper, only the vo
gets configured (lines 17-19)
step3) When the decoding starts, vd_theora.c::decode() gets called. We
now need an mpi image to do the decoding, so we call
mpcodecs_get_image() (line 26) with the following parameters:
mpi = mpcodecs_get_image(sh, MP_IMGTYPE_EXPORT, 0, width, height);
(note that the theora decoder allocates its buffers internally, so
we're requesting an image of type "export"). But mpcodecs_get_image()
now calls vf_get_image() (line 27), which recognizes that we need an
image of type '"export" (line 28) and creates a new mpi_image_t with
new_mp_image(w2,h); Here's the code of this function:
mp_image_t* new_mp_image(int w,int h){
mp_image_t* mpi = malloc(sizeof(mp_image_t));
if(!mpi) return NULL; // error!
memset(mpi,0,sizeof(mp_image_t));
mpi->width=mpi->w=w;
mpi->height=mpi->h=h;
return mpi;
}
So, when the mpi image is created, mpi->width=mpi->w=w; and
mpi->height=mpi->h=h; . At this point we call mp_image_setfmt() (line
29) which sets up our mp_image struct according to the out_fmt. In
particular, for planar yuv images does (among other things) the
following:
mpi->flags|=MP_IMGFLAG_YUV;
mpi->num_planes=3;
if (mp_get_chroma_shift(out_fmt, NULL, NULL)) {
mpi->flags|=MP_IMGFLAG_PLANAR;
mpi->bpp = mp_get_chroma_shift(out_fmt, &mpi->chroma_x_shift,
&mpi->chroma_y_shift);
mpi->chroma_width = mpi->width >> mpi->chroma_x_shift;
mpi->chroma_height = mpi->height >> mpi->chroma_y_shift;
}
So, with the help of mp_get_chroma_shift(), sets up mpi->chroma_width
and mpi->chroma_height to the proper values. This is why I used use
mpi->chroma_width and mpi->chroma_height in my patch. (If the image
dimensions/parameters don't change from now on, we're safe).
Then we get a message telling us that we're exporting the image (line
30) and finally the vo can work on the image (line 31)
step4) All the above gets repeated for the second frame (lines 33-35)
and third frame (lines 37-39), but this time everything is already set
up, so we don't call new_mp_image() and mp_image_setfmt(), and the
following values:
mpi->w
mpi->h
mpi->width
mpi->height
mpi->chroma_width
mpi->chroma_height
shoud remain constant. This means that it's always
(mpi->width==mpi->w) and (mpi->height==mpi->h), and that we should be
able to use mpi->chroma_width and mpi->chroma_height without problems.
Scenario 2: We insert a simple filter, like "-vf mirror", or another
filter that does not modify image dimensions: everything should still
work as expected. Log:
==========================================================================
Forced video codec: theora
Opening video decoder: [theora] Theora/VP3
theora: Theora video init ok!
theora: Frame 320x240, Picture 320x240, Offset [0,0]
VDec: vo config request - 320 x 240 (preferred colorspace: Planar 444P)
VDec: trying filter chain: mirror vo
VDec: vo_debug: query(Planar YV12,0x32315659) returned 0x403 (i=0)
VDec: vo_debug: codec query_format(Planar YV12,0x32315659) returned FALSE
VDec: vo_debug: query(Planar 422P,0x50323234) returned 0x403 (i=1)
VDec: vo_debug: codec query_format(Planar 422P,0x50323234) returned FALSE
VDec: vo_debug: query(Planar 444P,0x50343434) returned 0x403 (i=2)
VDec: vo_debug: codec query_format(Planar 444P,0x50343434) returned TRUE
VDec: using Planar 444P as output colorspace (n: 2)
Movie-Aspect is 1.33:1 - prescaling to correct movie aspect.
VDec: VO Config (320x240->320x240,flags=0,'MPlayer',0x50343434)
REQ: flags=0x3 req=0x0
VO: [md5sum] 320x240 => 320x240 Planar 444P
VO: Description: md5sum of each frame
VO: Author: Ivo van Poorten (ivop at euronet.nl)
Selected video codec: [theora] vfm: theora (Theora (free, reworked VP3))
==========================================================================
Audio: no sound
Freeing 0 unused audio chunks.
Starting playback...
theora: mpcodecs_get_image()
vf: vf_get_image(): [mirror] w=320 h=240, vf->w=320 vf->h=240
vf: imgtype=export
vf: mp_image_setfmt()
*** [mirror] Exporting mp_image_t, 320x240x24bpp YUV planar, 230400 bytes
vf: vf_get_image(): [vo] w=320 h=240, vf->w=320 vf->h=240
vf: imgtype=temp
vf: mp_image_setfmt()
vf: mp_image_alloc_planes()
*** [vo] Allocating mp_image_t, 320x240x24bpp YUV planar, 230400 bytes
vo_md5sum: draw_image() w=320 h=240
theora: mpcodecs_get_image()
vf: vf_get_image(): [mirror] w=320 h=240, vf->w=320 vf->h=240
vf: vf_get_image(): [vo] w=320 h=240, vf->w=320 vf->h=240
vo_md5sum: draw_image() w=320 h=240
theora: mpcodecs_get_image()
vf: vf_get_image(): [mirror] w=320 h=240, vf->w=320 vf->h=240
vf: vf_get_image(): [vo] w=320 h=240, vf->w=320 vf->h=240
vo_md5sum: draw_image() w=320 h=240
EOF code: 1
Uninit video: theora
vo: x11 uninit called but X11 not initialized..
Exiting... (End of file)
Worth noting: this time the vo need to allocate an image of type "temp"
Scenario 3: We insert the mirror filter and the expand filter that
modify image dimensions, like this "-vf mirror,expand=-80:-60": we
might have some problems here, I'm not sure. Log:
==========================================================================
Forced video codec: theora
Opening video decoder: [theora] Theora/VP3
theora: Theora video init ok!
theora: Frame 320x240, Picture 320x240, Offset [0,0]
VDec: vo config request - 320 x 240 (preferred colorspace: Planar 444P)
VDec: trying filter chain: mirror expand vo
VDec: vo_debug: query(Planar YV12,0x32315659) returned 0x403 (i=0)
VDec: vo_debug: codec query_format(Planar YV12,0x32315659) returned FALSE
VDec: vo_debug: query(Planar 422P,0x50323234) returned 0x403 (i=1)
VDec: vo_debug: codec query_format(Planar 422P,0x50323234) returned FALSE
VDec: vo_debug: query(Planar 444P,0x50343434) returned 0x403 (i=2)
VDec: vo_debug: codec query_format(Planar 444P,0x50343434) returned TRUE
VDec: using Planar 444P as output colorspace (n: 2)
Movie-Aspect is 1.33:1 - prescaling to correct movie aspect.
VDec: VO Config (320x240->320x240,flags=0,'MPlayer',0x50343434)
REQ: flags=0x403 req=0x0
REQ: flags=0x3 req=0x0
VO: [md5sum] 400x300 => 400x300 Planar 444P
VO: Description: md5sum of each frame
VO: Author: Ivo van Poorten (ivop at euronet.nl)
Selected video codec: [theora] vfm: theora (Theora (free, reworked VP3))
==========================================================================
Audio: no sound
Freeing 0 unused audio chunks.
Starting playback...
theora: mpcodecs_get_image()
vf: vf_get_image(): [mirror] w=320 h=240, vf->w=320 vf->h=240
vf: imgtype=export
vf: mp_image_setfmt()
*** [mirror] Exporting mp_image_t, 320x240x24bpp YUV planar, 230400 bytes
vf: vf_get_image(): [expand] w=320 h=240, vf->w=320 vf->h=240
vf: imgtype=temp
vf: mp_image_setfmt()
vf: vf_get_image(): [vo] w=400 h=300, vf->w=400 vf->h=300
vf: imgtype=temp
vf: mp_image_setfmt()
vf: mp_image_alloc_planes()
*** [vo] Allocating mp_image_t, 400x300x24bpp YUV planar, 360000 bytes
*** [expand] Direct Rendering mp_image_t, 400x240x24bpp YUV planar, 288000 bytes
vo_md5sum: draw_image() w=400 h=300
theora: mpcodecs_get_image()
vf: vf_get_image(): [mirror] w=320 h=240, vf->w=320 vf->h=240
vf: vf_get_image(): [expand] w=320 h=240, vf->w=320 vf->h=240
vf.c: MPI parameters changed! 400x240 -> 320x240
vf: vf_get_image(): [vo] w=400 h=300, vf->w=400 vf->h=300
vo_md5sum: draw_image() w=400 h=300
theora: mpcodecs_get_image()
vf: vf_get_image(): [mirror] w=320 h=240, vf->w=320 vf->h=240
vf: vf_get_image(): [expand] w=320 h=240, vf->w=320 vf->h=240
vf.c: MPI parameters changed! 400x240 -> 320x240
vf: vf_get_image(): [vo] w=400 h=300, vf->w=400 vf->h=300
vo_md5sum: draw_image() w=400 h=300
EOF code: 1
Uninit video: theora
vo: x11 uninit called but X11 not initialized..
Exiting... (End of file)
There are lines like this: "vf.c: MPI parameters changed! 400x240 ->
320x240", but also in vf.c there is this piece of code for handling
this case:
if(mpi->flags&MP_IMGFLAG_ALLOCATED){
if(mpi->width<w2 || mpi->height<h){
// need to re-allocate buffer memory:
av_free(mpi->planes[0]);
mpi->flags&=~MP_IMGFLAG_ALLOCATED;
mp_msg(MSGT_VFILTER,MSGL_V,"vf.c: have to REALLOCATE buffer memory :(\n");
}
// } else {
} {
mpi->width=w2; mpi->chroma_width=(w2 + (1<<mpi->chroma_x_shift) -
1)>>mpi->chroma_x_shift;
mpi->height=h; mpi->chroma_height=(h + (1<<mpi->chroma_y_shift) -
1)>>mpi->chroma_y_shift;
}
}
so it might work just fine.
Scenario N: I started with a particular decoder, theora, that exports
its buffer. But other decoders use direct rendering if I understand
correctly, so things may stop working/work differently and my patch
could be wrong. The cases are endless, and testing would take a lot of
time.
>> @@ -195,8 +195,8 @@
>> static uint32_t draw_image(mp_image_t *mpi)
>> {
>> unsigned char md5sum[16];
>> - uint32_t w = mpi->w;
>> - uint32_t h = mpi->h;
>> + uint32_t w = mpi->width;
>> + uint32_t h = mpi->height;
>
> And that this is more clear is a more than questionable claim IMO.
Ok.
> That is completely independent from the fact that such cosmetic changes
> (well, if it was one) would have to be in a separate patch, and in
> addition if they were the same the right solution would be to remove one,
> not randomly use one in some parts of the code and the other in other
> parts of the code.
Sorry, I agree.
>> @@ -215,8 +215,8 @@
>> for (i=0; i<h; i++) {
>> av_md5_update(md5_context, planeY + i * strideY, w);
>> }
>> - w = w / 2;
>> - h = h / 2;
>> + w = mpi->chroma_width;
>> + h = mpi->chroma_height;
>
> For the same reason you can't use these here, you'll have to explicitly
> shift by chroma_?_shift
See step3.
To cut a long story short, I should try to investigate this further,
but my knowledge of the codebase is limited, and right now so is my
time. Plus, the indentation in some parts of the code is _really_
messy, the documentation and comments within the code are pretty much
inexistent, and all this makes the life of someone who reads the code
for the first time really hard. So Reimar, if you think it could be an
useful addition, please fix it and commit it. If not, sorry for the
noise. Thank you.
Giorgio Vazzana
[1] https://trac.xiph.org/raw-attachment/ticket/1599/queens-444.ogv
More information about the MPlayer-dev-eng
mailing list