| Message ID | 20180211143752.7851-3-jeebjp@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Feb 11, 2018 at 04:37:51PM +0200, Jan Ekström wrote: > From: Masaki Tanaka <maki.rxrz@gmail.com> > > Utilizes the temporal_reference field from the picture header. > --- > libavcodec/mpegvideo_parser.c | 1 + > 1 file changed, 1 insertion(+) should be ok unless its intended to also restore the MSB Theres no code using output_picture_number currently thx [...]
On Sun, Feb 11, 2018 at 7:52 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > > should be ok unless its intended to also restore the MSB > The value seemed to be 10bit and if the pointer is at the position right after the picture_start_code, then `buf[0] << 2` would move the 8 bits left of data two bits leftwards, and `buf[1] >> 6` would then move the topmost bits of the following byte to be the bottom-most bits. Unless you mean that there's a forgotten `(uint16_t)` there as the end value has 10 bits of effective data, making the correct way of doing it something a la `(((uint16_t)buf[0]) << 2) | (((uint16_t)buf[1]) >> 6)`? > Theres no code using output_picture_number currently > Seems like some API users utilize this value. Jan
On Sun, Feb 11, 2018 at 8:31 PM, Jan Ekström <jeebjp@gmail.com> wrote: > On Sun, Feb 11, 2018 at 7:52 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: >> >> should be ok unless its intended to also restore the MSB >> > > The value seemed to be 10bit and if the pointer is at the position > right after the picture_start_code, then `buf[0] << 2` would move the > 8 bits left of data two bits leftwards, and `buf[1] >> 6` would then > move the topmost bits of the following byte to be the bottom-most > bits. > > Unless you mean that there's a forgotten `(uint16_t)` there as the end > value has 10 bits of effective data, making the correct way of doing > it something a la `(((uint16_t)buf[0]) << 2) | (((uint16_t)buf[1]) >> > 6)`? > Ping regarding this part. Jan
On Sun, Feb 11, 2018 at 08:31:40PM +0200, Jan Ekström wrote: > On Sun, Feb 11, 2018 at 7:52 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > should be ok unless its intended to also restore the MSB > > > > The value seemed to be 10bit and if the pointer is at the position > right after the picture_start_code, then `buf[0] << 2` would move the > 8 bits left of data two bits leftwards, and `buf[1] >> 6` would then > move the topmost bits of the following byte to be the bottom-most > bits. > > Unless you mean that there's a forgotten `(uint16_t)` there as the end > value has 10 bits of effective data, making the correct way of doing > it something a la `(((uint16_t)buf[0]) << 2) | (((uint16_t)buf[1]) >> > 6)`? it was more a question about our API if output_picture_number is intended to be what it is called in english then this patch is wrong as the number will frequently go back to 0 and as more than 1024 pictures are output. so taken as its english meaning, there would be code needed to increase(/decrease) the MSB occasionally OTOH if the field is intended to store a unspecified number of LSB only i wonder what it could be used for > > > Theres no code using output_picture_number currently > > > > Seems like some API users utilize this value. What do they do with it ? [...]
diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c index 3406346a8b..4f554b684e 100644 --- a/libavcodec/mpegvideo_parser.c +++ b/libavcodec/mpegvideo_parser.c @@ -60,6 +60,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s, switch(start_code) { case PICTURE_START_CODE: if (bytes_left >= 2) { + s->output_picture_number = (buf[0] << 2) | (buf[1] >> 6); s->pict_type = (buf[1] >> 3) & 7; if (bytes_left >= 4) vbv_delay = ((buf[1] & 0x07) << 13) | (buf[2] << 5) | (buf[3] >> 3);
From: Masaki Tanaka <maki.rxrz@gmail.com> Utilizes the temporal_reference field from the picture header. --- libavcodec/mpegvideo_parser.c | 1 + 1 file changed, 1 insertion(+)