GNOME Bugzilla – Bug 689420
Support new Vorbis/FLAC cover art specification
Last modified: 2014-02-24 18:20:39 UTC
As previously reported at: https://github.com/stsquad/easytag/issues/5 https://sourceforge.net/tracker/?func=detail&aid=2837960&group_id=5216&atid=105216 Please read http://wiki.xiph.org/index.php/VorbisComment#Cover_art for the new cover art comment format. In short, it is a base64 encoded version of the FLAC METADATA_BLOCK_PICTURE block.
Also filed at https://sourceforge.net/tracker/?func=detail&aid=3368703&group_id=5216&atid=355216
Created attachment 269889 [details] [review] This patch will add support for reading new Vorbis/FLAC cover art specification
Review of attachment 269889 [details] [review]: The patch needs to leave the old code for reading the COVERART field, as there will still be many files with that field. The Vorbis comment specification has good advice on how compatibility should be maintained: http://wiki.xiph.org/index.php/VorbisComment#Cover_art You also need to change some of the assumptions in the surrounding code (for instance, METADATA_BLOCK_PICTURE is now a supported field type). You should deal with the image writing support in a separate patch. ::: src/ogg_tag.c @@ +122,3 @@ + * + * Reads and returns an integer from given byte string starting from start. + * Returns: Integer which is read The integers in the FLAC picture struct are all uint32, so the return type here should also be guint32: https://xiph.org/flac/api/structFLAC____StreamMetadata__Picture.html @@ +125,3 @@ + */ +int +read_int_from_byte (guchar *str, int start) "start" should be a gsize. @@ +129,3 @@ + int i, read = 0; + for (i = start; i < start + 4; i++) + read = (read << 8) + str[i]; Even for single-line blocks, put them in curly braces. @@ +563,1 @@ * - COVERARTMIME : image/jpeg or image/png (written only) Update the comments near code that you change, if the code refers to that code, as otherwise the comment is wrong. @@ +567,3 @@ { Picture *pic; + int bytes_readed, mimelen, desclen; These should all be "gsize". "readed" is not a word, so "pos" (for "position") would be better. @@ +582,2 @@ // Decode picture data + decoded_ustr = g_base64_decode (string, &decoded_len); As g_base64_decode() already creates a duplicate of the data during the decoding process, you need to be careful to free the data after you are done with it. @@ +585,1 @@ + /*Reading picture type*/ Please add spaces between the asterisk and the text. @@ +595,3 @@ + bytes_readed += 4; + + pic->description = g_malloc (sizeof (guchar) * desclen); The result of sizeof (char) (and unsigned char) is defined to be 1 by the C standard, so the sizeof is unnecessary. @@ +597,3 @@ + pic->description = g_malloc (sizeof (guchar) * desclen); + for (i = 0; i < desclen; i++) + pic->description [i] = decoded_ustr [i + bytes_readed]; Even for single-line blocks, put them in curly braces (and indent by 4 columns). @@ +607,3 @@ + pic->data = g_malloc (sizeof (guchar) * pic->size); + for(i = 0; i < pic->size; i++) + pic->data [i] = decoded_ustr [i + bytes_readed]; Even for single-line blocks, put them in curly braces.
Created attachment 269937 [details] [review] This patch will add support for reading new Vorbis/FLAC cover art specification Did all the changes as said in the previous post.
Review of attachment 269937 [details] [review]: Thanks, that is looking a lot better. I ran EasyTAG under Valgrind with these changes, and there are some invalid reads that need fixing (they look like off-by-one errors): ==22157== Invalid read of size 1 ==22157== at 0x4A09304: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x54006B2: g_strdup (gstrfuncs.c:355) ==22157== by 0x451C49: Picture_Copy_One (picture.c:1091) ==22157== by 0x451CAD: Picture_Copy (picture.c:1103) ==22157== by 0x43BD81: ET_Add_File_To_File_List (et_core.c:3898) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== Address 0x19fef26e is 0 bytes after a block of size 14 alloc'd ==22157== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x53E8AC0: g_malloc (gmem.c:97) ==22157== by 0x44F95B: ogg_tag_read_file_tag (ogg_tag.c:634) ==22157== by 0x43BE7F: ET_Add_File_To_File_List (et_core.c:501) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== by 0x50A19EB: gdk_event_dispatch (gdkevents-x11.c:2403) ==22157== ==22157== Invalid read of size 1 ==22157== at 0x4A0A640: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x54006CC: g_strdup (gstrfuncs.c:357) ==22157== by 0x451C49: Picture_Copy_One (picture.c:1091) ==22157== by 0x451CAD: Picture_Copy (picture.c:1103) ==22157== by 0x43BD81: ET_Add_File_To_File_List (et_core.c:3898) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== Address 0x19fef26e is 0 bytes after a block of size 14 alloc'd ==22157== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x53E8AC0: g_malloc (gmem.c:97) ==22157== by 0x44F95B: ogg_tag_read_file_tag (ogg_tag.c:634) ==22157== by 0x43BE7F: ET_Add_File_To_File_List (et_core.c:501) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== by 0x50A19EB: gdk_event_dispatch (gdkevents-x11.c:2403) ==22157== ==22157== Invalid read of size 1 ==22157== at 0x5412BA4: _g_utf8_normalize_wc (gunidecomp.c:373) ==22157== by 0x5411EF1: g_utf8_collate (gunicollate.c:113) ==22157== by 0x43A996: ET_Detect_Changes_Of_File_Tag (et_core.c:4326) ==22157== by 0x43ADA5: ET_Manage_Changes_Of_File_Data (et_core.c:4130) ==22157== by 0x43BD97: ET_Add_File_To_File_List (et_core.c:647) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== Address 0x19fef26e is 0 bytes after a block of size 14 alloc'd ==22157== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x53E8AC0: g_malloc (gmem.c:97) ==22157== by 0x44F95B: ogg_tag_read_file_tag (ogg_tag.c:634) ==22157== by 0x43BE7F: ET_Add_File_To_File_List (et_core.c:501) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== by 0x50A19EB: gdk_event_dispatch (gdkevents-x11.c:2403) ==22157== ==22157== Invalid read of size 1 ==22157== at 0x5412CBB: _g_utf8_normalize_wc (gunidecomp.c:402) ==22157== by 0x5411EF1: g_utf8_collate (gunicollate.c:113) ==22157== by 0x43A996: ET_Detect_Changes_Of_File_Tag (et_core.c:4326) ==22157== by 0x43ADA5: ET_Manage_Changes_Of_File_Data (et_core.c:4130) ==22157== by 0x43BD97: ET_Add_File_To_File_List (et_core.c:647) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== Address 0x19fef26e is 0 bytes after a block of size 14 alloc'd ==22157== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x53E8AC0: g_malloc (gmem.c:97) ==22157== by 0x44F95B: ogg_tag_read_file_tag (ogg_tag.c:634) ==22157== by 0x43BE7F: ET_Add_File_To_File_List (et_core.c:501) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== by 0x50A19EB: gdk_event_dispatch (gdkevents-x11.c:2403) ==22157== ==22157== Invalid read of size 1 ==22157== at 0x304D8490CF: vfprintf (in /usr/lib64/libc-2.18.so) ==22157== by 0x304D874D52: vasprintf (in /usr/lib64/libc-2.18.so) ==22157== by 0x5424310: g_vasprintf (gprintf.c:316) ==22157== by 0x5403EAE: g_string_append_vprintf (gstring.c:1159) ==22157== by 0x540401E: g_string_printf (gstring.c:1228) ==22157== by 0x450C5F: Picture_Info (picture.c:923) ==22157== by 0x4520C9: PictureEntry_Update (picture.c:1023) ==22157== by 0x438427: ET_Display_File_Tag_To_UI (et_core.c:3117) ==22157== by 0x43D193: ET_Display_File_Data_To_UI (et_core.c:2680) ==22157== by 0x42F987: Action_Select_Nth_File_By_Etfile (easytag.c:1970) ==22157== by 0x41EB0C: Browser_Display_Tree_Or_Artist_Album_List (browser.c:2320) ==22157== by 0x432D95: Read_Directory (easytag.c:3398) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== Address 0x19fef26e is 0 bytes after a block of size 14 alloc'd ==22157== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x53E8AC0: g_malloc (gmem.c:97) ==22157== by 0x44F95B: ogg_tag_read_file_tag (ogg_tag.c:634) ==22157== by 0x43BE7F: ET_Add_File_To_File_List (et_core.c:501) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== by 0x50A19EB: gdk_event_dispatch (gdkevents-x11.c:2403) ==22157== ==22157== Invalid read of size 1 ==22157== at 0x4A09304: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x54006B2: g_strdup (gstrfuncs.c:355) ==22157== by 0x451C49: Picture_Copy_One (picture.c:1091) ==22157== by 0x4520D4: PictureEntry_Update (picture.c:1024) ==22157== by 0x438427: ET_Display_File_Tag_To_UI (et_core.c:3117) ==22157== by 0x43D193: ET_Display_File_Data_To_UI (et_core.c:2680) ==22157== by 0x42F987: Action_Select_Nth_File_By_Etfile (easytag.c:1970) ==22157== by 0x41EB0C: Browser_Display_Tree_Or_Artist_Album_List (browser.c:2320) ==22157== by 0x432D95: Read_Directory (easytag.c:3398) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== Address 0x19fef26e is 0 bytes after a block of size 14 alloc'd ==22157== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x53E8AC0: g_malloc (gmem.c:97) ==22157== by 0x44F95B: ogg_tag_read_file_tag (ogg_tag.c:634) ==22157== by 0x43BE7F: ET_Add_File_To_File_List (et_core.c:501) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== by 0x50A19EB: gdk_event_dispatch (gdkevents-x11.c:2403) ==22157== ==22157== Invalid read of size 1 ==22157== at 0x4A0A640: memcpy@@GLIBC_2.14 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x54006CC: g_strdup (gstrfuncs.c:357) ==22157== by 0x451C49: Picture_Copy_One (picture.c:1091) ==22157== by 0x4520D4: PictureEntry_Update (picture.c:1024) ==22157== by 0x438427: ET_Display_File_Tag_To_UI (et_core.c:3117) ==22157== by 0x43D193: ET_Display_File_Data_To_UI (et_core.c:2680) ==22157== by 0x42F987: Action_Select_Nth_File_By_Etfile (easytag.c:1970) ==22157== by 0x41EB0C: Browser_Display_Tree_Or_Artist_Album_List (browser.c:2320) ==22157== by 0x432D95: Read_Directory (easytag.c:3398) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== Address 0x19fef26e is 0 bytes after a block of size 14 alloc'd ==22157== at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22157== by 0x53E8AC0: g_malloc (gmem.c:97) ==22157== by 0x44F95B: ogg_tag_read_file_tag (ogg_tag.c:634) ==22157== by 0x43BE7F: ET_Add_File_To_File_List (et_core.c:501) ==22157== by 0x432C74: Read_Directory (easytag.c:3371) ==22157== by 0x41ABF2: Browser_Tree_Node_Selected (browser.c:715) ==22157== by 0x53582F6: _g_closure_invoke_va (gclosure.c:831) ==22157== by 0x53719FC: g_signal_emit_valist (gsignal.c:3215) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E3109D: gtk_tree_view_real_set_cursor (gtktreeview.c:12597) ==22157== by 0x4E355F7: gtk_tree_view_button_press (gtktreeview.c:2797) ==22157== by 0x4D3D9C4: _gtk_marshal_BOOLEAN__BOXED (gtkmarshalers.c:86) ==22157== by 0x53580C7: g_closure_invoke (gclosure.c:768) ==22157== by 0x536A24A: signal_emit_unlocked_R (gsignal.c:3589) ==22157== by 0x5372098: g_signal_emit_valist (gsignal.c:3317) ==22157== by 0x5372691: g_signal_emit (gsignal.c:3363) ==22157== by 0x4E4D3C3: gtk_widget_event_internal (gtkwidget.c:5010) ==22157== by 0x4D3C173: gtk_propagate_event (gtkmain.c:2490) ==22157== by 0x4D3C52A: gtk_main_do_event (gtkmain.c:1685) ==22157== by 0x50A19EB: gdk_event_dispatch (gdkevents-x11.c:2403)
Created attachment 269978 [details] [review] This patch will add support for reading new Vorbis/FLAC cover art specification Forgot to add null terminating character to description. Fixed!!
Review of attachment 269978 [details] [review]: OK, this gets rid of the Valgrind warnings, thanks. I have pushed a slightly-modified version of the patch to the wip/vorbis-coverart branch as commit aa80e7d67d0df0ebf0770fdd099020d77aaa9fbb. Before pushing to master, you should create a separate patch to write Vorbis cover art in the same format. Be sure to read the guidance in the Vorbis comment specification: http://wiki.xiph.org/index.php/VorbisComment#Conversion_to_METADATA_BLOCK_PICTURE
Created attachment 270052 [details] [review] This patch will add support for writing new Vorbis/FLAC cover art specification This patch should be applied to wip/vorbis-coverart branch of easytag.
Review of attachment 270052 [details] [review]: Rather than calling g_realloc when appending to the string, it would be better to either use GByteArray or to calculate the required size of the base64-encoded buffer at the beginning, then g_malloc() exactly that amount.
Created attachment 270126 [details] [review] This patch will add support for writing new Vorbis/FLAC cover art specification
Comment on attachment 269978 [details] [review] This patch will add support for reading new Vorbis/FLAC cover art specification I pushed the modified patch to master as commit bbcd91f4ca2e89ba36a02a0d9b21c23cba98090e.
Comment on attachment 270126 [details] [review] This patch will add support for writing new Vorbis/FLAC cover art specification I pushed a modified version of this patch to master (after fixing a memory leak) as commit 58e3d9489bd0ca38787b6b82b4a9321c52c51d22.
Additionally, I pushed a follow-up patch to mark files with old COVERART fields as modified, so that those files should be saved before exisiting (in a similar way to how ID3 tags can be converted between versions). Thanks for the patches!