Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#3900 closed defect (fixed)

ff_check_h264_startcode does not support 3-byte startcodes

Reported by: Lastique Owned by:
Priority: normal Component: avformat
Version: git-master Keywords: h264 avi
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

This is a regression from ffmpeg 2.2 series. The problem was reproduced with ffmpeg 2.3.3.

ff_check_h264_startcode (in libavformat/mpegtsenc.c) fails to recognize 3-byte h264 startcodes (i.e. 00 00 01), which results in errors while saving h264 content to some containers (avi in my case) with message "H.264 bitstream malformed, no startcode found, use the h264_mp4toannexb bitstream filter (-bsf h264_mp4toannexb)". The attached patch fixes the problem.

The 3-byte startcode is produced by libx264 for SEI NAL. Our application uses libx264 and ffmpeg through C API and at some point passes an AVPacket that starts with SEI to av_interleaved_write_frame, which fails.

Attachments (5)

07_fix_avformat_h264_startcode_check.patch (1.2 KB ) - added by Lastique 10 years ago.
The patch adds support for 3-byte h264 startcodes.
ticket3900.avi (334.4 KB ) - added by Lastique 10 years ago.
Sample media to reproduce the problem.
07_fix_avformat_startcode_checks.patch (2.2 KB ) - added by Lastique 9 years ago.
An updated patch that is compatible with 2.4.2 release and also fixes a similar check for HEVC.
3byte_startcode_test.cpp (9.6 KB ) - added by Lastique 9 years ago.
A testcase to reproduce the problem
test.ts (275.8 KB ) - added by Lastique 9 years ago.
Test MPEG TS file

Download all attachments as: .zip

Change History (24)

by Lastique, 10 years ago

The patch adds support for 3-byte h264 startcodes.

comment:1 by Carl Eugen Hoyos, 10 years ago

Please either:

  • Send your patch (made with git format-patch) to the FFmpeg developer mailing list where it can be discussed. Patches are often ignored on this bug tracker.

or

  • Please explain how I can reproduce this regression with current FFmpeg git head, posting a failing command line together with the complete, uncut console output.

by Lastique, 10 years ago

Attachment: ticket3900.avi added

Sample media to reproduce the problem.

comment:2 by Lastique, 10 years ago

You can try the following command:

ffmpeg -i ticket3900.avi -vcodec copy -an out.avi

It produces the follwoing output for me:

ffmpeg version 2.3.3 Copyright (c) 2000-2014 the FFmpeg developers
  built on Aug 28 2014 10:54:00 with gcc 4.8 (Ubuntu 4.8.2-19ubuntu1)
  configuration: --prefix=/usr --extra-cflags='-g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security ' --extra-ldflags='-Wl,-Bsymbolic-functions -Wl,-z,relro' --cc='ccache cc' --enable-shared --enable-libmp3lame --enable-gpl --enable-nonfree --enable-libvorbis --enable-pthreads --enable-libfaac --enable-libxvid --enable-postproc --enable-x11grab --enable-libgsm --enable-libtheora --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libx264 --enable-libspeex --enable-nonfree --disable-stripping --enable-libvpx --enable-libschroedinger --disable-encoder=libschroedinger --enable-version3 --enable-libopenjpeg --enable-librtmp --enable-avfilter --enable-libfreetype --enable-libvo-aacenc --disable-decoder=amrnb --enable-libvo-amrwbenc --enable-libaacplus --libdir=/usr/lib/x86_64-linux-gnu --disable-vda --enable-libbluray --enable-libcdio --enable-gnutls --enable-frei0r --enable-openssl --enable-libass --enable-libopus --enable-fontconfig --enable-libpulse --disable-mips32r2 --disable-mipsdspr1 --disable-mipsdspr2 --enable-libzvbi --enable-avresample --enable-libiec61883 --enable-libfdk-aac --enable-vaapi --enable-libx265 --enable-libdc1394 --disable-altivec --disable-armv5te --disable-armv6 --shlibdir=/usr/lib/x86_64-linux-gnu
  libavutil      52. 92.100 / 52. 92.100
  libavcodec     55. 69.100 / 55. 69.100
  libavformat    55. 48.100 / 55. 48.100
  libavdevice    55. 13.102 / 55. 13.102
  libavfilter     4. 11.100 /  4. 11.100
  libavresample   1.  3.  0 /  1.  3.  0
  libswscale      2.  6.100 /  2.  6.100
  libswresample   0. 19.100 /  0. 19.100
  libpostproc    52.  3.100 / 52.  3.100
Input #0, avi, from 'ticket3900.avi':
  Metadata:
    encoder         : Lavf55.48.100
  Duration: 00:00:20.22, start: 0.000000, bitrate: 135 kb/s
    Stream #0:0: Video: h264 (High) (H264 / 0x34363248), yuvj420p(pc), 320x240, 94 kb/s, 20 fps, 20 tbr, 20 tbn, 40 tbc
    Stream #0:1: Audio: aac ([255][0][0][0] / 0x00FF), 16000 Hz, mono, fltp, 128 kb/s
Output #0, avi, to 'out.avi':
  Metadata:
    ISFT            : Lavf55.48.100
    Stream #0:0: Video: h264 (H264 / 0x34363248), yuvj420p, 320x240, q=2-31, 94 kb/s, 20 fps, 20 tbn, 20 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (copy)
Press [q] to stop, [?] for help
[avi @ 0x10f8b20] H.264 bitstream malformed, no startcode found, use the h264_mp4toannexb bitstream filter (-bsf h264_mp4toannexb)
av_interleaved_write_frame(): Invalid argument
frame=    1 fps=0.0 q=-1.0 Lsize=       6kB time=00:00:00.05 bitrate= 915.2kbits/s    
video:6kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown
Conversion failed!

It succeeds when the patch is applied.

in reply to:  description ; comment:3 by Carl Eugen Hoyos, 10 years ago

Replying to Lastique:

The 3-byte startcode is produced by libx264 for SEI NAL. Our application uses libx264 and ffmpeg through C API and at some point passes an AVPacket that starts with SEI to av_interleaved_write_frame, which fails.

This seems to describe a different failure than the one for which you uploaded a sample and provided a command line. Is the issue with using libx264 to encode and libavformat to mux not reproducible with ffmpeg (the application)?

I tested your patch and the file you uploaded: The patch allows to write invalid transport streams with ffmpeg -i ticket3900.avi​ -vcodec copy out.ts which could be fixed by moving the changed function to libavformat/avienc.c but neither the original file nor the (with your patch) remuxed avi file play with WMP, so I suspect the error message is not wrong or do I miss something?

in reply to:  3 ; comment:4 by Lastique, 10 years ago

Replying to cehoyos:

Replying to Lastique:

The 3-byte startcode is produced by libx264 for SEI NAL. Our application uses libx264 and ffmpeg through C API and at some point passes an AVPacket that starts with SEI to av_interleaved_write_frame, which fails.

This seems to describe a different failure than the one for which you uploaded a sample and provided a command line. Is the issue with using libx264 to encode and libavformat to mux not reproducible with ffmpeg (the application)?

I don't use ffmpeg application in my case, so I can't tell. The test case I described simply illustrates the issue I'm having when using ffmpeg through C API. If you inspect the sample file you will notice that its first frame starts with a 3-byte startcode and a SEI NAL. I could create a C program to produce the same error, it's just more time consuming.

I tested your patch and the file you uploaded: The patch allows to write invalid transport streams with ffmpeg -i ticket3900.avi​ -vcodec copy out.ts which could be fixed by moving the changed function to libavformat/avienc.c but neither the original file nor the (with your patch) remuxed avi file play with WMP, so I suspect the error message is not wrong or do I miss something?

As far as I understand, the 3-byte startcode is valid in h264 (see ITU-T H.264, Annex B; there is also a shorter description here: http://www.szatmary.org/blog/25), so ffmpeg should recognize and support it. This is not specific to avi or any other container since this is about the NAL bitstream format. If WMP doesn't support the 3-byte startcode then WMP is buggy. That said, it may be that WMP doesn't support the particular combination of the codec, bitstream parameters and container. I'm not really concerned with WMP.

in reply to:  4 ; comment:5 by Carl Eugen Hoyos, 10 years ago

Replying to Lastique:

I don't use ffmpeg application in my case, so I can't tell. The test case I described simply illustrates the issue I'm having when using ffmpeg through C API.

To which container are you writing through the API?
Shouldn't it be easy to call x264 from ffmpeg with the same options as you do?

in reply to:  5 comment:6 by Lastique, 10 years ago

Replying to cehoyos:

To which container are you writing through the API?

To avi. But as far as I can tell, any container that uses ff_check_h264_startcode will have the same problem.

Shouldn't it be easy to call x264 from ffmpeg with the same options as you do?

I don't know ffmpeg command line very well but probably not. I tried to transcode an input file to h264 avi but ffmpeg muxes the first frame differently - it starts with SPS and PPS, each having the 4-byte startcode, and then goes SEI with the 3-byte startcode. Since ff_check_h264_startcode only checks the beginning of the frame, it doesn't fail. I don't know how to make ffmpeg application put SEI in a separate frame.

by Lastique, 9 years ago

An updated patch that is compatible with 2.4.2 release and also fixes a similar check for HEVC.

comment:7 by Carl Eugen Hoyos, 9 years ago

Generlly, send the patch to the ffmpeg development mailing list: Patches are ignored on this bug tracker and all review happens on the development mailinst list.
Remove the whitespace change from the patch (do not change the indentation of the unchanged lines, this can be done in a subsequent patch), commit the patch locally to current FFmpeg git head, and produce a file with git format-patch HEAD^ that you can send to the mailing list.

But as the patch would allow to write invalid transport streams, the more important question is of course: How can your issue - encoding with libx264 fails - be reproduced?

comment:8 by Lastique, 9 years ago

Generlly, send the patch to the ffmpeg development mailing list...

Thanks but the problem is I don't have much time to work on this issue, and I'd prefer not being involved into lengthy discussions on the ML.

But as the patch would allow to write invalid transport streams,...

Why do you consider streams with 3-byte start codes invalid?

the more important question is of course: How can your issue - encoding with libx264 fails - be reproduced?

I described it in the ticket. I can make a C/C++ test case using the API if that helps.

in reply to:  8 comment:9 by Carl Eugen Hoyos, 9 years ago

Replying to Lastique:

But as the patch would allow to write invalid transport streams,...

Why do you consider streams with 3-byte start codes invalid?

I don't consider it invalid, but all playback applications I tested did...

the more important question is of course: How can your issue - encoding with libx264 fails - be reproduced?

I described it in the ticket. I can make a C/C++ test case using the API if that helps.

A way to reproduce the problem will definitely help imo.

by Lastique, 9 years ago

Attachment: 3byte_startcode_test.cpp added

A testcase to reproduce the problem

comment:10 by Lastique, 9 years ago

The attached testcase reproduces the problem, it ends with these error messages:

[avi @ 0x19c7440] H.264 bitstream malformed, no startcode found, use the video bitstream filter 'h264_mp4toannexb' to fix it ('-bsf:v h264_mp4toannexb' option with ffmpeg)

and

Failure: Failed to write video frame, error: -1094995529

Note that you can see the encoded video packets if you compile it with -DDUMP_PACKETS. You can see then that the first packet contains the 3-byte start code.

comment:11 by Carl Eugen Hoyos, 9 years ago

Could you attach a short transport stream recorded with your application and your patch (so that it has a three byte start code)?

by Lastique, 9 years ago

Attachment: test.ts added

Test MPEG TS file

comment:12 by Lastique, 9 years ago

Our application doesn't normally write transport streams, but saving to a .ts file produced this file (attached). It doesn't play on my machine, even with libav and mplayer, which do support 3-byte startcodes.

comment:13 by Lastique, 9 years ago

BTW, you can also produce a TS file with the test case I attached by changing the file name.

comment:14 by Carl Eugen Hoyos, 9 years ago

Don't you agree that the file you attached indicates that your patch is not correct as-is since it allows to write invalid transport streams?
Please consider to send your patch (made with git format-patch) to the developer mailing list where people who actually know H.264 and MPEG transport streams (and avi) can comment, no patch review happens on this bug tracker.

comment:15 by Carl Eugen Hoyos, 9 years ago

Keywords: h264 avi added
Version: 2.3git-master

comment:16 by Kieran Kunhya, 9 years ago

That transport stream lacks SPS/PPS so that's why it doesn't play.

in reply to:  14 comment:17 by Lastique, 9 years ago

Replying to cehoyos:

Don't you agree that the file you attached indicates that your patch is not correct as-is since it allows to write invalid transport streams?

No, I don't. You didn't specify why exactly the patch is not correct except that "player X doesn't play such files". This is not a justification for me. What I'd like to see as a justification is a reference to a standard paper or a formal documentation of some sort stating that 3-byte start codes are not acceptable in this container or in these circumstances. Further, if such justification is shown, it should also be decided whether ffmpeg should perform the startcode conversion internally or not. I.e. if the limitation is container-specific, ffmpeg should handle this internally as a part of writing media file, IMHO.

Besides:

That transport stream lacks SPS/PPS so that's why it doesn't play.

Yes, that's probably the case. I don't really understand why they didn't get written though, the code is the same as it is for avi or other containers, and SPS/PPS were placed in the extradata.

comment:18 by Carl Eugen Hoyos, 9 years ago

Resolution: fixed
Status: newclosed

It appears that I "fixed" this issue in 56ffde3f619076650bdc6003cc683cf2f563463f

comment:19 by Lastique, 9 years ago

Thanks. Did you not fix the similar problem for HEVC in check_hevc_startcode?

Note: See TracTickets for help on using tickets.