Re: [libav-devel] [PATCH] h264: fix PCM intra-coded blocks in monochrome case

2011-08-19 Thread Ronald S. Bultje
Hi,

On Aug 19, 2011 7:04 AM, "Diego Biurrun"  wrote:
>
> From: Jeff Downs 
>
> Signed-off-by: Diego Biurrun 
> ---
>  libavcodec/h264.c |   46 ++
>  1 files changed, 34 insertions(+), 12 deletions(-)
>
> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index 4feef4b..5116ca2 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -1824,15 +1824,30 @@ static av_always_inline void
hl_decode_mb_internal(H264Context *h, int simple, i
> tmp_y[j] = get_bits(&gb, bit_depth);
> }
> if(simple || !CONFIG_GRAY || !(s->flags&CODEC_FLAG_GRAY)){
> -for (i = 0; i < 8; i++) {
> -uint16_t *tmp_cb = (uint16_t*)(dest_cb +
i*uvlinesize);
> -for (j = 0; j < 8; j++)
> -tmp_cb[j] = get_bits(&gb, bit_depth);
> -}
> -for (i = 0; i < 8; i++) {
> -uint16_t *tmp_cr = (uint16_t*)(dest_cr +
i*uvlinesize);
> -for (j = 0; j < 8; j++)
> -tmp_cr[j] = get_bits(&gb, bit_depth);
> +if (!h->sps.chroma_format_idc) {
> +for (i = 0; i < 8; i++) {
> +uint16_t *tmp_cb = (uint16_t*)(dest_cb +
i*uvlinesize);
> +for (j = 0; j < 8; j++) {
> +tmp_cb[j] = 1 << (bit_depth - 1);
> +}
> +}
> +for (i = 0; i < 8; i++) {
> +uint16_t *tmp_cr = (uint16_t*)(dest_cr +
i*uvlinesize);
> +for (j = 0; j < 8; j++) {
> +tmp_cr[j] = 1 << (bit_depth - 1);
> +}
> +}
> +} else {
> +for (i = 0; i < 8; i++) {
> +uint16_t *tmp_cb = (uint16_t*)(dest_cb +
i*uvlinesize);
> +for (j = 0; j < 8; j++)
> +tmp_cb[j] = get_bits(&gb, bit_depth);
> +}
> +for (i = 0; i < 8; i++) {
> +uint16_t *tmp_cr = (uint16_t*)(dest_cr +
i*uvlinesize);
> +for (j = 0; j < 8; j++)
> +tmp_cr[j] = get_bits(&gb, bit_depth);
> +}
> }
> }
> } else {
> @@ -1840,9 +1855,16 @@ static av_always_inline void
hl_decode_mb_internal(H264Context *h, int simple, i
> memcpy(dest_y + i*  linesize, h->mb   + i*8, 16);
> }
> if(simple || !CONFIG_GRAY || !(s->flags&CODEC_FLAG_GRAY)){
> -for (i=0; i<8; i++) {
> -memcpy(dest_cb+ i*uvlinesize, h->mb + 128 + i*4,  8);
> -memcpy(dest_cr+ i*uvlinesize, h->mb + 160 + i*4,  8);
> +if (!h->sps.chroma_format_idc) {
> +for (i = 0; i < 8; i++) {
> +memset(dest_cb + i*uvlinesize, 128, 8);
> +memset(dest_cr + i*uvlinesize, 128, 8);
> +}
> +} else {
> +for (i = 0; i < 8; i++) {
> +memcpy(dest_cb + i*uvlinesize, h->mb + 128 + i*4,
 8);
> +memcpy(dest_cr + i*uvlinesize, h->mb + 160 + i*4,
 8);
> +}
> }
> }
> }

Lgtm.

Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] h264: fix PCM intra-coded blocks in monochrome case

2011-08-17 Thread Ronald S. Bultje
Hi,

On Wed, Aug 17, 2011 at 9:57 AM, Diego Biurrun  wrote:
> On Wed, Aug 17, 2011 at 09:44:39AM -0700, Ronald S. Bultje wrote:
>>
>> On Wed, Aug 17, 2011 at 9:17 AM, Diego Biurrun  wrote:
>> >             if(simple || !CONFIG_GRAY || !(s->flags&CODEC_FLAG_GRAY)){
>> > +                uint16_t *tmp_cb, *tmp_cr;
>> >                 for (i = 0; i < 8; i++) {
>> > -                    uint16_t *tmp_cb = (uint16_t*)(dest_cb + 
>> > i*uvlinesize);
>> > -                    for (j = 0; j < 8; j++)
>> > -                        tmp_cb[j] = get_bits(&gb, bit_depth);
>> > +                    tmp_cb = (uint16_t*)(dest_cb + i*uvlinesize);
>> > +                    tmp_cr = (uint16_t*)(dest_cr + i*uvlinesize);
>> >                 }
>>
>> You understand that whatever happens below this is out of context and
>> thus this code cannot possibly work, right?
>
> I'm not sure I'm following you, the scope of the variables is increased
> here, so I'm missing the issue you are hinting at.  IIUC what you mean
> by "out of context", i.e. out of scope, then the code would not even
> compile, which it does...

variable x, y;
for (i = 0; i < 8; i++) {
x = i;
y = i;
}

for (i = 0; i < 8; i++) {
for (j = 0; j < 8; j++) {
// do something with x;
}
}

for (i = 0; i < 8; i++) {
for (j = 0; j < 8; j++) {
// do something with y;
}
}

do you understand why this doesn't do what the below code does?

for (i = 0; i < 8; i++) {
x = i;
for (j = 0; j < 8; j++) {
// do something with x;
}
}

// and then same for y here

Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] h264: fix PCM intra-coded blocks in monochrome case

2011-08-17 Thread Diego Biurrun
On Wed, Aug 17, 2011 at 09:44:39AM -0700, Ronald S. Bultje wrote:
> 
> On Wed, Aug 17, 2011 at 9:17 AM, Diego Biurrun  wrote:
> >             if(simple || !CONFIG_GRAY || !(s->flags&CODEC_FLAG_GRAY)){
> > +                uint16_t *tmp_cb, *tmp_cr;
> >                 for (i = 0; i < 8; i++) {
> > -                    uint16_t *tmp_cb = (uint16_t*)(dest_cb + i*uvlinesize);
> > -                    for (j = 0; j < 8; j++)
> > -                        tmp_cb[j] = get_bits(&gb, bit_depth);
> > +                    tmp_cb = (uint16_t*)(dest_cb + i*uvlinesize);
> > +                    tmp_cr = (uint16_t*)(dest_cr + i*uvlinesize);
> >                 }
> 
> You understand that whatever happens below this is out of context and
> thus this code cannot possibly work, right?

I'm not sure I'm following you, the scope of the variables is increased
here, so I'm missing the issue you are hinting at.  IIUC what you mean
by "out of context", i.e. out of scope, then the code would not even
compile, which it does...

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] h264: fix PCM intra-coded blocks in monochrome case

2011-08-17 Thread Ronald S. Bultje
Hi,

On Wed, Aug 17, 2011 at 9:17 AM, Diego Biurrun  wrote:
>             if(simple || !CONFIG_GRAY || !(s->flags&CODEC_FLAG_GRAY)){
> +                uint16_t *tmp_cb, *tmp_cr;
>                 for (i = 0; i < 8; i++) {
> -                    uint16_t *tmp_cb = (uint16_t*)(dest_cb + i*uvlinesize);
> -                    for (j = 0; j < 8; j++)
> -                        tmp_cb[j] = get_bits(&gb, bit_depth);
> +                    tmp_cb = (uint16_t*)(dest_cb + i*uvlinesize);
> +                    tmp_cr = (uint16_t*)(dest_cr + i*uvlinesize);
>                 }

You understand that whatever happens below this is out of context and
thus this code cannot possibly work, right?

Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] h264: fix PCM intra-coded blocks in monochrome case

2011-08-17 Thread Diego Biurrun
On Wed, Aug 17, 2011 at 05:52:53PM +0200, Diego Biurrun wrote:
> From: Jeff Downs 

Discard this one, mistakenly sent without local changes.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel