Jeff Downs <[email protected]> added the comment:
On Sun, 2 Aug 2009, Michael Niedermayer wrote:
> On Wed, Jul 29, 2009 at 09:25:56PM +0000, Jeff Downs wrote:
>
> there are so any mb_y checks, why dont you fix one of them instead of adding
> yet another.
> slice_decode_thread contains a check it can be moved up
Not sure what you mean by moving this one up. slice_decode_thread is
re-reading from the input bitstream, so it needs to re-check the input
and also know when its reached the slice to stop reading.
... unless we want to get in to bounding the threaded reading code to
checked input data rather than bounding by start code/mb_y ranges.
> decode_chunks contains one limited to mpeg2
That one implements -skip_bottom. Currently we emit warnings for bad
slice numbers, but we wouldn't want to emit warnings for skipped mb rows.
Attached moves the mb_y check (which emits the warning) in decode_slice up
to decode_chunk. For non-threaded decoding, behavior will be the same.
For threaded decoding, it fixes the original issue, makes certain the
thread context's end_mb_y is valid, and also fixes the thread context
allocation for field pictures. One less check is performed at threaded
decoding time, at the expense of the extra check at thread allocation
time.
Also noticed that the skip_bottom code is wrong for field pics, but that's
a separate issue.
_____________________________________________________
FFmpeg issue tracker <[email protected]>
<https://roundup.ffmpeg.org/roundup/ffmpeg/issue1277>
_____________________________________________________
Index: libavcodec/mpeg12.c
===================================================================
--- libavcodec/mpeg12.c (revision 19605)
+++ libavcodec/mpeg12.c (working copy)
@@ -1690,11 +1690,6 @@
s->resync_mb_x=
s->resync_mb_y= -1;
- if (mb_y<<field_pic >= s->mb_height){
- av_log(s->avctx, AV_LOG_ERROR, "slice below image (%d >= %d)\n", mb_y,
s->mb_height);
- return -1;
- }
-
init_get_bits(&s->gb, *buf, buf_size*8);
ff_mpeg1_clean_buffers(s);
@@ -2362,6 +2357,8 @@
if (start_code >= SLICE_MIN_START_CODE &&
start_code <= SLICE_MAX_START_CODE) {
int mb_y= start_code - SLICE_MIN_START_CODE;
+ int mb_h= s2->mb_height >>
+ (s2->picture_structure != PICT_FRAME);
if(s2->last_picture_ptr==NULL){
/* Skip B-frames if we do not have reference frames and gop is
not closed */
@@ -2396,6 +2393,11 @@
break;
}
+ if (mb_y >= mb_h){
+ av_log(avctx, AV_LOG_ERROR, "slice below image (%d >=
%d)\n", mb_y, mb_h);
+ break;
+ }
+
if(!s2->pict_type){
av_log(avctx, AV_LOG_ERROR, "Missing picture start
code\n");
break;
@@ -2417,12 +2419,12 @@
}
if(avctx->thread_count > 1){
- int threshold= (s2->mb_height*s->slice_count +
avctx->thread_count/2) / avctx->thread_count;
+ int threshold= (mb_h*s->slice_count +
avctx->thread_count/2) / avctx->thread_count;
if(threshold <= mb_y){
MpegEncContext *thread_context=
s2->thread_context[s->slice_count];
thread_context->start_mb_y= mb_y;
- thread_context->end_mb_y = s2->mb_height;
+ thread_context->end_mb_y = mb_h;
if(s->slice_count){
s2->thread_context[s->slice_count-1]->end_mb_y=
mb_y;
ff_update_duplicate_context(thread_context, s2);
