[Bug 728845] Option to supply input media-files from a playlist-file for gst-play tool

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=728845
  GStreamer | gst-plugins-base | git

Tim-Philipp Müller <t.i.m> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #275020|none                        |needs-work
             status|                            |

--- Comment #2 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2014-04-26 10:02:22 UTC ---
(From update of attachment 275020)
Thanks for the patch. This looks like a good addition.

>+  gchar *list = NULL;

Please rename to playlist_file or playlist_fn or so.

>+    {"playlist", 0, 0, G_OPTION_ARG_STRING, &list,
>+        N_("Playlist file containing input media files"), NULL},

I think this should be a G_OPTION_ARG_FILENAME instead.

>+#define MAX_PATH_LEN 512
>+  FILE *playlistfd = NULL;
>+  gchar line[MAX_PATH_LEN];

We prefer not to hardcode lengths like this if not needed, but rather do a few
extra string copies, at least in tools like this, see below.

>+  if (list != NULL) {
>+    playlistfd = fopen (list, "r");
>+
>+    if (playlistfd != NULL) {
>+      while (fgets (line, MAX_PATH_LEN, playlistfd) != NULL) {
>+        if (strlen (line) - 1) {
>+          /* Ignore line feed */
>+          line[strlen (line) - 1] = 0;
>+          add_to_playlist (playlist, line);
>+        }
>+      }
>+      fclose (playlistfd);
>+    }
>+  }

Could you rewrite this bit using g_file_get_contents() + g_strsplit()?

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.