[FFmpeg-devel] [PATCH]yuv4mpeg: Do not set interlaced_frame
Michael Niedermayer
michaelni at gmx.at
Fri Feb 1 23:26:35 CET 2013
On Wed, Jan 30, 2013 at 12:43:44AM +0100, Carl Eugen Hoyos wrote:
> On Tuesday 29 January 2013 05:57:58 pm Tim Nicholson wrote:
> > On 29/01/13 09:18, Carl Eugen Hoyos wrote:
> > > Hi!
> > >
> > > The documentation of interlaced_frame says that it should only be written
> > > by libavcodec, the yuv4mpeg demuxer currently violates this.
does this fix an actual bug or ticket or is it just a API docs based
fix ?
> > >
> > > Attached patch fixes this and changes the behaviour on damaged headers
> > > slightly.
> > >
> > > Please review, Carl Eugen
> > >
> > >
> > > patchyuv4mpeginterlaced.diff
> > >
> > >
> > > diff --git a/libavformat/yuv4mpeg.c b/libavformat/yuv4mpeg.c
> > > index ff039bd..0eaa48c 100644
> > > --- a/libavformat/yuv4mpeg.c
> > > +++ b/libavformat/yuv4mpeg.c
> > > @@ -58,6 +58,12 @@ static int yuv4_generate_header(AVFormatContext *s,
> > > char* buf) inter = 'p'; /* progressive is the default */
> > > if (st->codec->coded_frame &&
> > > st->codec->coded_frame->interlaced_frame) inter =
> > > st->codec->coded_frame->top_field_first ? 't' : 'b'; + if
> > > (st->codec->field_order > AV_FIELD_UNKNOWN)
> > > + if (st->codec->field_order > AV_FIELD_PROGRESSIVE) {
> > > + inter = st->codec->field_order == AV_FIELD_TT ||
> > > st->codec->field_order == AV_FIELD_TB ? 't' : 'b'; + } else {
> > > + inter = 'p';
> > > + }
> >
> > So you are leaving the original as a fall back when
> > (st->codec->field_order == AV_FIELD_UNKNOWN ?
>
> Yes, applications may rely on that behaviour and it does not seem
> unreasonable to me.
>
> [...]
>
> > > + switch (interlaced){
> > > + case 'p':
> > > + st->codec->field_order = AV_FIELD_PROGRESSIVE;
> > > + break;
> > > + case 't':
> > > + st->codec->field_order = AV_FIELD_TT;
> >
> > I believe this should be AV_FIELD_TB
> >
> > > + break;
> > > + case 'b':
> > > + st->codec->field_order = AV_FIELD_BB;
> >
> > and this AV_FIELD_BT
>
> Both changed.
>
> Thank you for the review!
>
> New split patches attached, Carl Eugen
> yuv4mpeg.c | 57 +++++++++++++++++++++++++++++----------------------------
> 1 file changed, 29 insertions(+), 28 deletions(-)
> 7cf0d47f5a30ffdb195972399ac730cacfaa9007 patchyuv4mpeginterlaced2.diff
> diff --git a/libavformat/yuv4mpeg.c b/libavformat/yuv4mpeg.c
> index 7d49b7a..3969f7f 100644
> --- a/libavformat/yuv4mpeg.c
> +++ b/libavformat/yuv4mpeg.c
> @@ -58,6 +58,12 @@ static int yuv4_generate_header(AVFormatContext *s, char* buf)
> inter = 'p'; /* progressive is the default */
> if (st->codec->coded_frame && st->codec->coded_frame->interlaced_frame)
> inter = st->codec->coded_frame->top_field_first ? 't' : 'b';
> + if (st->codec->field_order > AV_FIELD_UNKNOWN)
> + if (st->codec->field_order > AV_FIELD_PROGRESSIVE) {
> + inter = st->codec->field_order == AV_FIELD_TT || st->codec->field_order == AV_FIELD_TB ? 't' : 'b';
> + } else {
> + inter = 'p';
> + }
>
> switch (st->codec->pix_fmt) {
> case AV_PIX_FMT_GRAY8:
> @@ -319,7 +325,7 @@ static int yuv4_read_header(AVFormatContext *s)
> {
> char header[MAX_YUV4_HEADER + 10]; // Include headroom for
> // the longest option
> - char *tokstart, *tokend, *header_end;
> + char *tokstart, *tokend, *header_end, interlaced = '?';
> int i;
> AVIOContext *pb = s->pb;
> int width = -1, height = -1, raten = 0,
> @@ -425,27 +431,7 @@ static int yuv4_read_header(AVFormatContext *s)
> tokstart++;
> break;
> case 'I': // Interlace type
> - switch (*tokstart++){
> - case '?':
> - break;
> - case 'p':
> - s1->interlaced_frame = 0;
> - break;
> - case 't':
> - s1->interlaced_frame = 1;
> - s1->top_field_first = 1;
> - break;
> - case 'b':
> - s1->interlaced_frame = 1;
> - s1->top_field_first = 0;
> - break;
> - case 'm':
> - av_log(s, AV_LOG_ERROR, "YUV4MPEG stream contains mixed "
> - "interlaced and non-interlaced frames.\n");
> - break;
> - default:
> - av_log(s, AV_LOG_ERROR, "YUV4MPEG has invalid header.\n");
> - }
> + interlaced = *tokstart++;
> break;
> case 'F': // Frame rate
> sscanf(tokstart, "%d:%d", &raten, &rated); // 0:0 if unknown
> @@ -546,6 +532,27 @@ static int yuv4_read_header(AVFormatContext *s)
> st->sample_aspect_ratio = (AVRational){ aspectn, aspectd };
> st->codec->chroma_sample_location = chroma_sample_location;
>
> + switch (interlaced){
> + case 'p':
> + st->codec->field_order = AV_FIELD_PROGRESSIVE;
> + break;
> + case 't':
> + st->codec->field_order = AV_FIELD_TB;
> + break;
> + case 'b':
> + st->codec->field_order = AV_FIELD_BT;
> + break;
> + case 'm':
> + av_log(s, AV_LOG_ERROR, "YUV4MPEG stream contains mixed "
> + "interlaced and non-interlaced frames.\n");
> + case '?':
> + st->codec->field_order = AV_FIELD_UNKNOWN;
> + break;
> + default:
> + av_log(s, AV_LOG_ERROR, "YUV4MPEG has invalid header.\n");
> + st->codec->field_order = AV_FIELD_UNKNOWN;
> + }
> +
> return 0;
> }
>
> @@ -555,7 +562,6 @@ static int yuv4_read_packet(AVFormatContext *s, AVPacket *pkt)
> char header[MAX_FRAME_HEADER+1];
> int packet_size, width, height, ret;
> AVStream *st = s->streams[0];
> - struct frame_attributes *s1 = s->priv_data;
>
> for (i = 0; i < MAX_FRAME_HEADER; i++) {
> header[i] = avio_r8(s->pb);
> @@ -587,11 +593,6 @@ static int yuv4_read_packet(AVFormatContext *s, AVPacket *pkt)
> else if (ret != packet_size)
> return s->pb->eof_reached ? AVERROR_EOF : AVERROR(EIO);
>
> - if (st->codec->coded_frame) {
> - st->codec->coded_frame->interlaced_frame = s1->interlaced_frame;
> - st->codec->coded_frame->top_field_first = s1->top_field_first;
> - }
I think this leaves some unused fields
also did you check that these AVFrame fields still get correctly
set after the decoder with this patch ?
[...]
> yuv4mpeg.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 1d1f91378634a390894503a48e63169bf4472396 patchyuv4mpegnofail.diff
> diff --git a/libavformat/yuv4mpeg.c b/libavformat/yuv4mpeg.c
> index ff039bd..7d49b7a 100644
> --- a/libavformat/yuv4mpeg.c
> +++ b/libavformat/yuv4mpeg.c
> @@ -442,10 +442,9 @@ static int yuv4_read_header(AVFormatContext *s)
> case 'm':
> av_log(s, AV_LOG_ERROR, "YUV4MPEG stream contains mixed "
> "interlaced and non-interlaced frames.\n");
> - return -1;
> + break;
ok
> default:
> av_log(s, AV_LOG_ERROR, "YUV4MPEG has invalid header.\n");
> - return -1;
does this fix an actual file ?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130201/84298c52/attachment.asc>
More information about the ffmpeg-devel
mailing list