Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Release video buffer #7

Closed
gpalsingh opened this issue Jul 29, 2017 · 9 comments
Closed

Release video buffer #7

gpalsingh opened this issue Jul 29, 2017 · 9 comments
Labels
h264d

Comments

@gpalsingh
Copy link
Owner

The code for releasing allocated buffers is commented out as of now https://github.com/gpalsingh/mesa/blob/gsoc-dev/src/gallium/state_trackers/omx_tizonia/h264dprc.c#L78

Not sure if it is possible or is even needed since the dec works.
@CapOM
Copy link

CapOM commented Jul 29, 2017

I think you just need to call video_buffer->destroy(video_buffer) in hash_table_clear_item_callback.
This is useful when restarting the pipeline or for resolution change so that we do not leak bigs images :)

@gpalsingh
Copy link
Owner Author

Made the changes in 5ee3bf3

It clears the video but I the error

*** Error in `/home/gpalsingh/gst/master/gstreamer/tools/.libs/lt-gst-launch-1.0': corrupted double-linked list: 0x00007f81d8128300 ***

Also while debugging I see that four eglimages are registered. egl_image_validation_hook is called 5 times and returns OMX_TRUE all times. This might be related to this since we were expecting 4 eglimages?

@CapOM
Copy link

CapOM commented Jul 29, 2017

I double check and 5 looks correct because:

min is 1 here https://github.com/gpalsingh/gst-omx/blob/tizonia-dev/omx/gstomxvideodec.c#L608
in min + port->port_def.nBufferCountMin
because of gstgl requires set it as 1 here:
https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst-libs/gst/gl/gstglupload.c#n398
And port->port_def.nBufferCountMin is 4 as set here: https://github.com/gpalsingh/mesa/blob/gsoc-dev/src/gallium/state_trackers/omx_tizonia/h264d.h#L16

But this would lead to 5 registered eglimage so this needs more digging if you see only 4.

Could you debug around "min = MAX (min + port->port_def.nBufferCountMin, 4);" in gstomxvideodec.c and check what is the value for each var ?

@gpalsingh
Copy link
Owner Author

Ah that looks right because I see 5 too.
So there might be some error in clearing the video buffers that leads to crash.

@gpalsingh gpalsingh added the h264d label Aug 3, 2017
@CapOM
Copy link

CapOM commented Aug 4, 2017

Same question here, does it happen with and without tee element ?

@gpalsingh
Copy link
Owner Author

The crash doesn't happen while using tee.
Gst pipeline: MESA_ENABLE_OMX_EGLIMAGE=1 GST_GL_API=gles2 GST_GL_PLATFORM=egl gst-launch-1.0 filesrc location=~/mp4_tests/10\ Second\ countdown.mp4.mp4 ! qtdemux ! h264parse ! omxh264dectiz ! tee ! glimagesink
But removing 'tee' makes the error appear again

@CapOM
Copy link

CapOM commented Aug 4, 2017

Ah right, my mistake since this code is only for eglimage

CapOM pushed a commit to CapOM/mesa that referenced this issue Aug 15, 2017
h264dprc.c: increment reference counter of the resource texture
377c62d 
Because vl_video_buffer_create_ex2 takes ownership.
And some minor cleanups.

In this commit only pipe_resource_reference(&resources[0], p_res);
fixes the crash.

Fixes gpalsingh/mesa#7
@CapOM
Copy link

CapOM commented Aug 15, 2017

Fixed by CapOM/mesa@377c62d , please cherry-pick this commit into your branch. We were just passing the texture without incrementing the reference counter. Which results in a double free when terminating the pipeline.

@gpalsingh
Copy link
Owner Author

Works too. Thx!

gpalsingh pushed a commit that referenced this issue Aug 16, 2017
@gpalsingh
h264dprc.c: increment reference counter of the resource texture
93023ad 
Because vl_video_buffer_create_ex2 takes ownership.
And some minor cleanups.

In this commit only pipe_resource_reference(&resources[0], p_res);
fixes the crash.

Fixes #7
This was referenced Aug 23, 2017
Closed
Open
gpalsingh pushed a commit that referenced this issue Oct 4, 2017
@nhaehnle
glsl: fix glsl_struct_field size calculations for shader cache
4da6cf6 
Found by address sanitizer:

==22621==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61400000cbd8 at pc 0x7f561610a4ff bp 0x7ffca85f9d50 sp 0x7ffca85f94f8
READ of size 344 at 0x61400000cbd8 thread T0
    #0 0x7f561610a4fe  (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x5f4fe)
    #1 0x7f560bb305a5 in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
    #2 0x7f560bb305a5 in blob_write_bytes ../../../mesa-src/src/compiler/glsl/blob.c:136
    #3 0x7f560be7d7ff in encode_type_to_blob ../../../mesa-src/src/compiler/glsl/shader_cache.cpp:153
    #4 0x7f560be81222 in write_program_resource_data ../../../mesa-src/src/compiler/glsl/shader_cache.cpp:950
    #5 0x7f560be81222 in write_program_resource_list ../../../mesa-src/src/compiler/glsl/shader_cache.cpp:1118
    #6 0x7f560be81222 in shader_cache_write_program_metadata(gl_context*, gl_shader_program*) ../../../mesa-src/src/compiler/glsl/shader_cache.cpp:1407
    #7 0x7f560b825fdb in link_program ../../../mesa-src/src/mesa/main/shaderapi.c:1163

Fixes: 073a84f ("glsl: stop adding pointers from glsl_struct_field to the cache")
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
gpalsingh pushed a commit that referenced this issue Oct 4, 2017
@gpalsingh
h264dprc.c: increment reference counter of the resource texture
85aca4d 
Because vl_video_buffer_create_ex2 takes ownership.
And some minor cleanups.

In this commit only pipe_resource_reference(&resources[0], p_res);
fixes the crash.

Fixes #7
gpalsingh pushed a commit that referenced this issue Oct 9, 2017
@gpalsingh
h264dprc.c: increment reference counter of the resource texture
7ee9153 
Because vl_video_buffer_create_ex2 takes ownership.
And some minor cleanups.

In this commit only pipe_resource_reference(&resources[0], p_res);
fixes the crash.

Fixes #7
gpalsingh pushed a commit that referenced this issue Oct 10, 2017
@gpalsingh
h264dprc.c: increment reference counter of the resource texture
6c0c774 
Because vl_video_buffer_create_ex2 takes ownership.
And some minor cleanups.

In this commit only pipe_resource_reference(&resources[0], p_res);
fixes the crash.

Fixes #7
gpalsingh pushed a commit that referenced this issue Nov 21, 2017
@gpalsingh
h264dprc.c: increment reference counter of the resource texture
7383982 
Because vl_video_buffer_create_ex2 takes ownership.
And some minor cleanups.

In this commit only pipe_resource_reference(&resources[0], p_res);
fixes the crash.

Fixes #7
gpalsingh pushed a commit that referenced this issue Nov 28, 2017
@gpalsingh
h264dprc.c: increment reference counter of the resource texture
c4d5e31 
Because vl_video_buffer_create_ex2 takes ownership.
And some minor cleanups.

In this commit only pipe_resource_reference(&resources[0], p_res);
fixes the crash.

Fixes #7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
h264d
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@CapOM @gpalsingh