After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 519460 - 8 bytes missing in AVI streamheader
8 bytes missing in AVI streamheader
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.x
Other Linux
: Normal major
: 0.10.9
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-02-29 07:28 UTC by Yang Hong
Modified: 2008-06-27 15:26 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
patch to fix this bug (745 bytes, patch)
2008-02-29 07:42 UTC, Yang Hong
rejected Details | Review

Description Yang Hong 2008-02-29 07:28:07 UTC
"gst-libs/gst/riff/riff-ids.h" 455 lines

/* common data structures */
typedef struct _gst_riff_strh {
  guint32 type;             /* stream type */
  guint32 fcc_handler;       /* fcc_handler */
  guint32 flags;
/* flags values */
#define GST_RIFF_STRH_DISABLED        0x000000001
#define GST_RIFF_STRH_VIDEOPALCHANGES 0x000010000
  guint32 priority;
  guint32 init_frames;       /* initial frames (???) */
  guint32 scale;
  guint32 rate;
  guint32 start;
  guint32 length;
  guint32 bufsize;           /* suggested buffer size */
  guint32 quality;
  guint32 samplesize;
  /* rcFrame, RECT structure(struct of 4 shorts)
  gint32  left;
  gint32  top;
  gint32  right;
  gint32  bottom;
  */
} gst_riff_strh;

The missed last 16byte in this header cause a AVI file recorded in gstreamer can't be played in most player in the MS Windows. The Xine or MPlayer does OK because it's stronger.:)
Comment 1 Sebastian Dröge (slomo) 2008-02-29 07:32:18 UTC
What exactly do you mean? There should be 16 more bytes in the struct? What are they good for? :)
Comment 2 Yang Hong 2008-02-29 07:41:12 UTC
(In reply to comment #1)
> What exactly do you mean? 
> There should be 16 more bytes in the struct?

Yes. some players in MS windows can't play such a video without out the 16 bytes.

> What are they good for? :)
> 

Add 16 bytes by remove The comment /* rcFrame... */.
Comment 3 Yang Hong 2008-02-29 07:42:00 UTC
Created attachment 106226 [details] [review]
patch to fix this bug
Comment 4 Sebastian Dröge (slomo) 2008-02-29 07:48:34 UTC
Ah... problem is, that this breaks ABI unfortunately :/

No idea what to do...
Comment 5 Yang Hong 2008-02-29 08:16:27 UTC
At least, we know why such a AVI can't play with those players.

Leave this issue for distribution, they will decide whether adapt this patch.
Comment 6 Wim Taymans 2008-02-29 17:45:05 UTC
you could just not touch the struct but write the extra bytes directly in avimux after writing the header. maybe not nice but it's a solution.
Comment 7 Tim-Philipp Müller 2008-02-29 17:55:28 UTC
And/or add a new _full struct with the additional bytes deriving from the current one (all these structs are a bit ugly in their current form anyway).
Comment 8 Wang Diancheng 2008-03-03 08:57:09 UTC
I try to fix this bug using its Comment #6 method.
but now there is a problem, when video reording finished, how to write total frames to "gst_riff_avih.dtot_frames" and "gst_riff_strh.length" which locate at start of avi file?

can I write something to start of file when state changed to GST_STATE_READY?

now, these fields are just filled with zero, but the video can not be played by player on MS Windows platform if their values are not correct. 
Comment 9 Wim Taymans 2008-03-03 10:57:36 UTC
Comment #6; avimux should currently write out the header when it receives EOS. It does this by sending a newsegment event to cause a seek in filesink back to the start and then overwriting the previous dummy header. If you're testing this from gst-launch, you wont be able to generate a proper EOS to test this behaviour (it does work ok when transcoding or when setting the num-buffers property on the source).
Comment 10 Wang Diancheng 2008-03-04 02:19:48 UTC
hi, thanks for your reply, but when I use num-buffers property to test it, it raise a CRITICAL error, following is the output:

#gst-launch-0.10 v4l2src num-buffers=10 queue-size=2 ! \ image/jpeg,width=320,height=240 !  avimux ! filesink location=x.avi

Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock

(gst-launch-0.10:9378): GStreamer-CRITICAL **: gst_segment_set_newsegment_full: assertion `segment->format == format' failed
Got EOS from element "pipeline0".
Execution ended after 1119148498 ns.
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
FREEING pipeline ...
Comment 11 Wang Diancheng 2008-03-04 03:11:18 UTC
following is the gdb backtrace:

  • #6 gst_segment_set_newsegment_full
    at gstsegment.c line 450
  • #7 gst_base_sink_configure_segment
    at gstbasesink.c line 1111
  • #8 gst_base_sink_render_object
    at gstbasesink.c line 2078
  • #9 gst_base_sink_queue_object_unlocked
    at gstbasesink.c line 2223
  • #10 gst_base_sink_event
    at gstbasesink.c line 2359
  • #11 gst_pad_send_event
    at gstpad.c line 4271
  • #12 gst_pad_push_event
    at gstpad.c line 4127
  • #13 gst_avi_mux_stop_file
    at gstavimux.c line 1459
  • #14 gst_avi_mux_collect_pads
    at gstavimux.c line 1647
  • #15 gst_collect_pads_check_collected
    at gstcollectpads.c line 954
  • #16 gst_collect_pads_event
    at gstcollectpads.c line 1054
  • #17 gst_avi_mux_handle_event
    at gstavimux.c line 1503
  • #18 gst_pad_send_event
    at gstpad.c line 4271
  • #19 gst_pad_push_event
    at gstpad.c line 4127
  • #20 gst_base_transform_sink_event
    at gstbasetransform.c line 1246
  • #21 gst_pad_send_event
    at gstpad.c line 4271
  • #22 gst_pad_push_event
    at gstpad.c line 4127
  • #23 gst_base_src_loop
    at gstbasesrc.c line 2157
  • #24 gst_task_func
    at gsttask.c line 192

Comment 12 Wang Diancheng 2008-03-04 04:57:44 UTC
This bug caused by following lines in gst_avi_mux_stop_file:gstavimux.c,

 event = gst_event_new_new_segment (FALSE, 1.0, GST_FORMAT_BYTES,
      avimux->total_data, GST_CLOCK_TIME_NONE, avimux->total_data);
->  gst_pad_push_event (avimux->srcpad, event);

why push a total_data event after push avi header event? 
Comment 13 Tim-Philipp Müller 2008-03-04 11:19:44 UTC
> This bug caused by following lines in gst_avi_mux_stop_file:gstavimux.c,
> 
>  event = gst_event_new_new_segment (FALSE, 1.0, GST_FORMAT_BYTES,
>       avimux->total_data, GST_CLOCK_TIME_NONE, avimux->total_data);
> ->  gst_pad_push_event (avimux->srcpad, event);
> 
> why push a total_data event after push avi header event? 

This should make the downstream filesink seek to the specified start position. I haven't looked at the code in question, but I'd guess this is done before sending the buffer with the index.
Comment 14 Wang Diancheng 2008-05-17 06:01:25 UTC
I checked gstreamer-plugins-good trunk, the comment #10 issue have been fixed, 
because "missed 16byte in AVI header" only affects helix, I think this bug can be closed.
Comment 15 Sebastian Dröge (slomo) 2008-05-17 10:10:32 UTC
Hm, I can't find a change in avimux that could've fixed it. Which one do you think was it?
Comment 16 Wang Diancheng 2008-05-17 10:56:34 UTC
I think maybe these changes:

2008-05-13  Mark Nauwelaerts  <mnauw@users.sf.net>

        * gst/avi/gstavimux.c: (gst_avi_mux_start_file):
        Send an initial BYTE segment to inform downstream of later seeking,
        and to forego sync attempts.

2008-05-12  Mark Nauwelaerts  <mnauw@users.sf.net>

        * gst/avi/gstavimux.c: (gst_avi_mux_pad_reset):
        Do not leave fourcc stream header field empty upon reset.
        Fixes #519301.
Comment 17 Mark Nauwelaerts 2008-05-17 12:06:38 UTC
The former of the above commits should have fixed the segment warning, as it should prevent sinks from doing fancy/nasty stuff with segments, such as forcing the format to TIME and complaining later on about BYTE seeks (see also bug 519878).  The other one is unrelated.

In any case, neither deals with any extra bytes in the header.
MS specs mainly intend these rcFrame fields for use with for several video streams, in which case it indicates the rectangle within the movie rectangle specified by the main (avi) header.  As such, I suspect another "solution" (e.g. mencoder) is to simply add 2 (int32) 0 (size gives the others).  But even as it stands, I have known the resulting files to play on Windows (IIRC including WMP), as also indicated by other reports.  And one would assume these players essentially come down to the same (MS) libs.

In view of this, how does "only affects Helix" (on Windows?) in Comment #10 relate to "some players in MS windows" in Comment #2, and that in turn to "can't be played in most player in the MS Windows" in the original report ?
Comment 18 Wang Diancheng 2008-05-19 01:41:38 UTC
(In reply to comment #17)
> The former of the above commits should have fixed the segment warning, as it
> should prevent sinks from doing fancy/nasty stuff with segments, such as
> forcing the format to TIME and complaining later on about BYTE seeks (see also
> bug 519878).  The other one is unrelated.
> 
> In any case, neither deals with any extra bytes in the header.
> MS specs mainly intend these rcFrame fields for use with for several video
> streams, in which case it indicates the rectangle within the movie rectangle
> specified by the main (avi) header.  As such, I suspect another "solution"
> (e.g. mencoder) is to simply add 2 (int32) 0 (size gives the others).  But even
> as it stands, I have known the resulting files to play on Windows (IIRC
> including WMP), as also indicated by other reports.  And one would assume these
> players essentially come down to the same (MS) libs.
> 
> In view of this, how does "only affects Helix" (on Windows?) in Comment #10
> relate to "some players in MS windows" in Comment #2, and that in turn to
> "can't be played in most player in the MS Windows" in the original report ?

sorry, it is my mistake, only Helix(on GNU/Linux) can not play files missing 16 bytes in AVI header. MS Windows Media Player is OK. 

> 

Comment 19 Mark Nauwelaerts 2008-06-27 15:26:13 UTC
2008-06-27  Mark Nauwelaerts  <mark.nauwelaerts@collabora.co.uk>

	* gst/avi/gstavimux.c: (gst_avi_mux_riff_get_avi_header):
	* gst/avi/gstavimux.h:
	Add 8 bytes to current streamheader to make for a complete one
	and to make more players happy.  Fixes #519460.