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 748700 - avimux: stopping file without index fails
avimux: stopping file without index fails
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.8.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-04-30 12:10 UTC by Jesper Larsen
Modified: 2016-05-13 08:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
avimux: Check for NULL index before writing it (809 bytes, patch)
2015-04-30 12:10 UTC, Jesper Larsen
none Details | Review
Test case (2.43 KB, text/x-csrc)
2015-04-30 12:11 UTC, Jesper Larsen
  Details
avimux: Do not write index and header if idx is NULL (829 bytes, patch)
2015-04-30 12:50 UTC, Jesper Larsen
none Details | Review

Description Jesper Larsen 2015-04-30 12:10:39 UTC
Created attachment 302641 [details] [review]
avimux: Check for NULL index before writing it

Sending eos to avimux with a video sink pad that has not received any buffers causes a critical:
GStreamer-CRITICAL **: gst_memory_new_wrapped: assertion 'data != NULL' failed

The problem is that the muxer tries to write the index, even though it is not available.

  • #0 g_logv
    at gmessages.c line 1046
  • #1 g_log
    at gmessages.c line 1079
  • #2 g_return_if_fail_warning
    at gmessages.c line 1088
  • #3 gst_memory_new_wrapped
  • #4 gst_avi_mux_write_index
    at gstavimux.c line 1601
  • #5 gst_avi_mux_stop_file
    at gstavimux.c line 1789
  • #6 gst_avi_mux_do_one_buffer
    at gstavimux.c line 2190
  • #7 gst_avi_mux_collect_pads
    at gstavimux.c line 2207
  • #8 gst_collect_pads_check_collected
    at gstcollectpads.c line 1323
  • #9 gst_collect_pads_event_default
    at gstcollectpads.c line 1767
  • #10 gst_avi_mux_handle_event
    at gstavimux.c line 1926
  • #11 gst_collect_pads_event
    at gstcollectpads.c line 1989
  • #12 gst_pad_send_event_unchecked
    at gstpad.c line 5358
  • #13 gst_pad_send_event
    at gstpad.c line 5525
  • #14 sighandle
    at avimux-test.c line 44
  • #15 <signal handler called>
  • #16 poll
    at ../sysdeps/unix/syscall-template.S line 81
  • #17 g_main_context_poll
    at gmain.c line 4103
  • #18 g_main_context_iterate
    at gmain.c line 3803
  • #19 g_main_loop_run
    at gmain.c line 4002
  • #20 main
    at avimux-test.c line 100

I have a test case, where I create the pipeline:

videotestsrc ! x264enc ! avimux ! fakesink

I put a blocking probe on the src pad of x264enc, and drop the first buffer showing up. avimux never receives any buffers.

On SIGINT I send an EOS event to the avimux sinkpad, which causes the assertion.

I have attached a patch that checks for a valid index before writing the index from gst_avi_mux_stop_file.
Maybe we want to skip rewriting the header altogether since we are dealing with an empty file anyway?
Comment 1 Jesper Larsen 2015-04-30 12:11:26 UTC
Created attachment 302642 [details]
Test case
Comment 2 Jesper Larsen 2015-04-30 12:50:30 UTC
Created attachment 302648 [details] [review]
avimux: Do not write index and header if idx is NULL

Actually, there are some other bugs in the rewriting of the header if using audio, so it might indeed be best to skip the rewriting of the header.

The other bug I found is a FPE because of:
avipad->parent.hdr.rate = avipad->auds.av_bps / avipad->auds.blockalign;

  • #0 gst_avi_mux_audsink_set_fields
    at gstavimux.c line 731
  • #1 gst_avi_mux_stop_file
    at gstavimux.c line 1827
  • #2 gst_avi_mux_do_one_buffer
    at gstavimux.c line 2190
  • #3 gst_avi_mux_collect_pads
    at gstavimux.c line 2207
  • #4 gst_collect_pads_check_collected
    at gstcollectpads.c line 1323
  • #5 gst_collect_pads_event_default
    at gstcollectpads.c line 1767
  • #6 gst_avi_mux_handle_event
    at gstavimux.c line 1926
  • #7 gst_collect_pads_event
    at gstcollectpads.c line 1989
  • #8 gst_pad_send_event_unchecked
    at gstpad.c line 5358
  • #9 gst_pad_send_event
    at gstpad.c line 5525

Comment 3 Tim-Philipp Müller 2016-05-13 08:39:19 UTC
Easy way to reproduce:

gst-launch-1.0 videotestsrc num-buffers=1 ! video/x-raw,width=16,height=16 ! identity drop-probability=1.0 ! avimux ! fakesink
Comment 4 Tim-Philipp Müller 2016-05-13 08:59:16 UTC
Pushed, thanks for the patch, and sorry it fell through the cracks!

commit ce05adfb30cbb48da5f9d3226f4fb0172684fe26
Author: Jesper Larsen <knorr.jesper@gmail.com>
Date:   Thu Apr 30 14:43:04 2015 +0200

    avimux: Do not write index and header if idx is NULL
    
    Fixes criticals with e.g.
    videotestsrc num-buffers=1 ! identity drop-probability=1.0 ! avimux ! fakesink
    
    https://bugzilla.gnome.org/show_bug.cgi?id=748700