Bugzilla is not a support forum.

What the critical is helpfully telling you is that the buffer is not writable as required when adding meta's to a buffer.  You can ensure writablility with gst_buffer_make_writable().

Can you provide a testcase that allows reproducing this problem? Are you certain that this warning comes from avdec_h264 and not your h264parse changes (try running with G_DEBUG=fatal_warnings and get a backtrace of that warning)?

(In reply to Sebastian Dröge (slomo) from comment #2)
> Can you provide a testcase that allows reproducing this problem? Are you
> certain that this warning comes from avdec_h264 and not your h264parse
> changes (try running with G_DEBUG=fatal_warnings and get a backtrace of that
> warning)?

Sebastian,

The test case is, I have created a metadata with custom SEI message that needs to be added after encode.

The "GStreamer-CRITICAL" message is from gst_buffer_add_h264_meta() function. This function uses the buffer sent through gst_video_decoder_finish_frame(). Please refer below gdb back trace.

************************************

G_DEBUG=fatal_warnings gdb --args  gst-launch-1.0 filesrc location=in.ts ! tsdemux ! h264parse ! 'video/x-h264, stream-format=byte-stream' ! avdec_h264 ! x264enc ! mpegtsmux ! filesink location=out.ts

(gdb) r
Starting program: /usr/local/bin/gst-launch-1.0 filesrc location=in.ts \! tsdemux \! h264parse \! video/x-h264,\ stream-format=byte-stream \! avdec_h264 \! x264enc \! mpegtsmux \! filesink location=out.ts
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib64/libthread_db.so.1".
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Redistribute latency...
Redistribute latency...
Redistribute latency...

(gst-launch-1.0:20583): GStreamer-CRITICAL **: gst_buffer_add_meta: assertion 'gst_buffer_is_writable (buffer)' failed

Program received signal SIGTRAP, Trace/breakpoint trap.

+ Trace 236879

** (gst-launch-1.0:9874): CRITICAL **: gst_video_frame_map_id: assertion 'GST_IS_BUFFER (buffer)' failed
ERROR: from element /GstPipeline:pipeline0/avdec_h264:avdec_h264-0: Cannot access memory for read and write operation.
Additional debug info:
gstavviddec.c(865): gst_ffmpegviddec_get_buffer2 (): /GstPipeline:pipeline0/avdec_h264:avdec_h264-0:
The video memory allocated from downstream pool could not mapped forread and write.
Execution ended after 0:00:00.3756


Regards,
Parithi

That's in your own meta. Please provide a testcase with code that we can run. This is most likely a bug in your meta implementation.

Created attachment 340682 [details]
H264 video meta test files

Sebatian,

I've attached my meta test code. And the changes I did in  gst-plugins-bad\gst\videoparsers\gsth264parse.c is below.

In gst_h264_parse_pre_push_frame() function, after the statement
       buffer = frame->buffer;

I've added the following code

  {
      guint length = strlen("Test H264 video meta");
      gst_buffer_add_h264_test_meta(buffer, length, "Test H264 video meta");
  }

You can use any .ts file with h264 elementary stream for testing.

Your copy function is wrong and will cause segfaults when both the copy and the original are freed. You have to also create a copy of the payload, e.g. with g_memdup(). That's probably the reason for the other problems.

Apart from that, this might be https://bugzilla.gnome.org/show_bug.cgi?id=740222

Sebastian,
Yes, the copying may cause segfault as you mentioned. But here, that's not the cause for the issue I mentioned. The issue ('gst_buffer_is_writable (buffer)' failed) exists even if there is no payload element in the structure.

I tried the same pipeline with "direct-rendering=false", it works properly.

As per GstVideoDecoder documentation, after calling gst_video_decoder_finish_frame() function, the output buffer of the frame is to be considered read-only.

With direct-rendering=true, which is the default value, output buffer is becoming read-only even before calling gst_video_decoder_finish_frame(). Because of this gst_buffer_add_h264_meta() function, which is called inside gst_video_decoder_finish_frame(), is failing.

Thanks for taking the time to report this.
This particular bug has already been reported into our bug tracking system, but please feel free to report any further bugs you find.

*** This bug has been marked as a duplicate of bug 740222 ***

I'm not convinced of the resolution is ideal. It's not fully the fact that the memory are mapped read-write, but the fact that the GstBuffer has a refcount of two. It's somehow a side effect, but in your case, calling gst_buffer_make_writable() is free, it will only create a new GstBuffer and ref the memory.

Of course you'll be doing more allocation, but it will work without serious performance (this is low rate, since it's video). To me, the bug is that your new code is not calling make_writable(), the result is not optimal, the remaining would require finding a workaround to the way libav works.

Was wondering the same, why we can't just call copy or make writable here.

GstVideoDecoder/Encoder should make sure the buffer is writable already when it tries to copy over metas. However as the buffer is mapped writable, a full memory copy will also be done here then, or not?

I must say, I just don't remember, would have to test it. Though, I realize that doing so will break the pooling.

Nicolas/Tim,

If I make the buffer writable by adding below code just before gst_video_decoder_finish_frame() call

   if(!gst_buffer_is_writable(out_frame->output_buffer))
   {
         gst_buffer_make_writable(out_frame->output_buffer);
   }


Then I am getting following error



(gst-launch-1.0:10963): GStreamer-CRITICAL **: gst_mini_object_unref: assertion 'mini_object->refcount > 0' failed

** (gst-launch-1.0:10963): CRITICAL **: gst_video_frame_map_id: assertion 'GST_IS_BUFFER (buffer)' failed
ERROR: from element /GstPipeline:pipeline0/avdec_h264:avdec_h264-0: Cannot access memory for read and write operation.
Additional debug info:
gstavviddec.c(865): gst_ffmpegviddec_get_buffer2 (): /GstPipeline:pipeline0/avdec_h264:avdec_h264-0:

The video memory allocated from downstream pool could not mapped forread and write.
Execution ended after 0:00:00.2103

That's because gst_buffer_make_writable() takes ownership of whatever you pass to it and returns a new buffer (or the same if nothing was needed).

The general pattern is "out_frame->output_buffer = gst_buffer_make_writable (out_frame->output_buffer)". You also don't need to check for writability first, the function does nothing if not needed.

And ref to maintain the baseclass referencence which you don't own.