GNOME Bugzilla – Bug 753920
dvdlpcmdec: factor out common code to set output format into new function
Last modified: 2015-09-26 08:31:34 UTC
Created attachment 309796 [details] [review] dvdlpcmdec: add new function to setting format for audio Dear All. To setting format for audio is performed by when caps is received or parsing header in dvdlpcmdec. I fixed to add a new function to support common logic. Please check and review this patch.
Please, don't change the importance mark, this is reserved for maintainers usage only.
Comment on attachment 309796 [details] [review] dvdlpcmdec: add new function to setting format for audio Please provide a more descriptive commit message, saying that you added a new function doesn't really explain anything. Say why you did that (e.g. "Refactor caps negotiation into its own function") :) Otherwise looks good
Created attachment 309938 [details] [review] Make a new function for setting valid channel and format Dear Sebastian and All. Thanks for your comment and help. As your comment, I updated commit message. Please refer new patch file and review it.
Created attachment 309944 [details] [review] Factor out common code to set output format into new function Dear All I updated commit message. Please review it again.
Thanks. When updating an existing patch in bugzilla, please also mark the previous version of the patch as obsolete by ticking the checkbox next to the old patch when attaching the new one, thanks. I have pushed this with three very minor changes: - prefixed update_audio_formats with gst_dvdlpcmdec_ - added the bug url to the commit message at the end - changed your clear-text name in 'Author' commit d7f78391b4490b896df2f06d7c73676a7fc616f6 Author: HoonHee Lee <hoonhee.lee@lge.com> Date: Tue Aug 25 10:08:46 2015 +0900 dvdlpcmdec: factor out common code to set output format into new function When caps event is recieved and header is changed, reordering channel and setting the default output format for audio are processed. These 2 of code are same. Thus, It is better to make a new function for these common code in terms of removing duplicated code, maintenance and expansion. https://bugzilla.gnome.org/show_bug.cgi?id=753920 I also noticed this while looking at the patch: commit d57b0971987598c73ce96c9026d422c94712caae Author: Tim-Philipp Müller <tim@centricular.com> Date: Sat Sep 26 09:23:05 2015 +0100 dvdlpcmdec: fix invalid read beyond channel position array We would always copy sizeof(sorted_position) bytes, which is for 8 channels, but if we have less than 8 channels the position array we copy from will only have allocated space for channel channels, so we would read beyond the input array in some cases.