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 458629 - [avidemux] high memory usage for many index entries
[avidemux] high memory usage for many index entries
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 0.10.17
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-07-20 12:41 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2009-10-07 20:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
conditionaly disable index copying (8.97 KB, patch)
2007-07-20 12:57 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
conditionaly disable index copying (10.11 KB, patch)
2007-09-03 10:39 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
conditionaly disable index copying (10.05 KB, patch)
2007-10-26 08:00 UTC, Tommi Myöhänen
needs-work Details | Review
patch adaptation for plugins-good-0.10.15 (6.74 KB, patch)
2009-07-01 09:33 UTC, Benjamin Gaignard
none Details | Review
same patch but with .h file patched (7.55 KB, patch)
2009-07-01 09:45 UTC, Benjamin Gaignard
none Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2007-07-20 12:41:40 UTC
Avidemux builds the index as a list and then sorts it into an array. This leads to a hight peak memory usage. Clips can easily use 20 mb for the index. So we touch 40 mb of memory before even going to playing. Imho working on the list is just fine. Seeking is not the usual usecase and going from one entry to the next is equaly fast with a sorted list.
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2007-07-20 12:57:18 UTC
Created attachment 92040 [details] [review]
conditionaly disable index copying
Comment 2 Stefan Sauer (gstreamer, gtkdoc dev) 2007-09-03 10:39:51 UTC
Created attachment 94855 [details] [review]
conditionaly disable index copying

Little cleanup of the patch.
Comment 3 Tommi Myöhänen 2007-10-26 08:00:10 UTC
Created attachment 97890 [details] [review]
conditionaly disable index copying

Updated patch against cvs head.

Any comments about the patch?
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2008-06-10 14:10:02 UTC
The patch needs updates once again, as this commit totally broke it :/

Revision 1.231 - (view) (download) (as text) (annotate) - [select for diffs]
Mon Jun 2 16:09:59 2008 UTC (7 days, 21 hours ago) by wtay
Branch: MAIN
Changes since 1.230: +152 -48 lines
Diff to previous 1.230

Patch by: Thijs Vermeir <thijsvermeir at gmail dot com>
* gst/avi/gstavidemux.c: (gst_avi_demux_index_next),
(gst_avi_demux_index_prev), (gst_avi_demux_index_entry_for_time),
(gst_avi_demux_do_seek), (gst_avi_demux_handle_seek),
(gst_avi_demux_process_next_entry):
* gst/avi/gstavidemux.h:
Implement reverse playback. Fixes #535300.
Comment 5 Marc-Andre Lureau 2008-08-27 12:35:45 UTC
marking patch as obsolete
Comment 6 Manish Sharma 2008-11-17 17:50:08 UTC
(In reply to comment #3)
> Created an attachment (id=97890) [edit]
> conditionaly disable index copying
> Updated patch against cvs head.
> Any comments about the patch?

I was trying to study this patch but observed a strange part in between.
at line number 85 "gst_avi_demux_index_last" function is implemented for GST_AVI_DISABLE_INDEX_ARRAY macro. But "gst_avi_demux_index_next" function implementation is missing is it ok?

Comment 7 Manish Sharma 2008-11-18 05:18:36 UTC
(In reply to comment #3)
> Created an attachment (id=97890) [edit]
> conditionaly disable index copying
> Updated patch against cvs head.
> Any comments about the patch?

I have applied the above patch on my gst-plugins-good 0.10.10 package.
Earlier I was not able to play huge files like 600MB due to memory constraint. After applying this patch I am able to play 600MB files.
Please let me know whether I can use this patch or will it give some other kind of defect?
Why this patch is marked as obsolete here?
Comment 8 Stefan Sauer (gstreamer, gtkdoc dev) 2008-11-18 07:16:19 UTC
Manish, you could apply the patch? It was marked as obsolete, because it did not apply to CVS anymore. If you have corrected it, can you regenerate the patch and reapply? I think the idea behind the patch is still valid.
Comment 9 Manish Sharma 2008-11-18 10:06:20 UTC
(In reply to comment #8)
> Manish, you could apply the patch? It was marked as obsolete, because it did
> not apply to CVS anymore. If you have corrected it, can you regenerate the
> patch and reapply? I think the idea behind the patch is still valid.

No! I was facing some issue while applying the patch. Right now I have taken the changes manually. I need to create a patch against the Head version to update on CVS. I will try to update it on CVS after testing.
Comment 10 Benjamin Gaignard 2009-07-01 09:33:52 UTC
Created attachment 137676 [details] [review]
patch adaptation for plugins-good-0.10.15
Comment 11 Benjamin Gaignard 2009-07-01 09:45:20 UTC
Created attachment 137677 [details] [review]
same patch but with .h file patched
Comment 12 Edward Hervey 2009-07-01 09:50:19 UTC
Benjamin, using a GList instead of an array will take more memory and will be slower to parse than using an array... so I don't really understand your patch.
Comment 13 Benjamin Gaignard 2009-07-01 11:24:30 UTC
it's removing one copy of the entries in gst_avi_demux_massage_index function.
To be honest I have only updated the obsolete patch to adapt it for plugins-good-0.10.15

Comment 14 Stefan Sauer (gstreamer, gtkdoc dev) 2009-07-01 15:25:11 UTC
Edward, the po nt of the patch is to avoid the peak memory consution that current avidemux has. It copies the list to the array and thus needs the double memory for a short moment. Also the list is not the performance bottleneck. There is one more indirection in searching. We could use a GSequence maybe to get a binary search (when the patch was made, there was no binary search in the array either).
Comment 15 Stefan Sauer (gstreamer, gtkdoc dev) 2009-10-07 20:51:41 UTC
This is now obsolete with the avidemux rewrite