[Merge] lp:~lorn-potter/media-hub/fix-1420728 into lp:media-hub
Jim Hodapp
jim.hodapp at canonical.com
Thu Jun 16 13:13:16 UTC 2016
Review: Needs Fixing code
Don't forget to set the commit message and description.
A few fix suggestions inline below.
Diff comments:
> === modified file 'src/core/media/gstreamer/playbin.cpp'
> --- src/core/media/gstreamer/playbin.cpp 2016-06-08 16:54:31 +0000
> +++ src/core/media/gstreamer/playbin.cpp 2016-06-16 00:15:21 +0000
> @@ -639,8 +639,30 @@
> if (!info)
> return std::string();
>
> - return std::string(g_file_info_get_attribute_string(
> - info.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE));
> + std::string content_type = g_file_info_get_attribute_string(info.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE);
This line might be longer than 100 chars as well, please break.
Also, const std::string
> + if (content_type.empty())
> + {
> + return std::string();
The style of the other code is to not use {} around a one line if-statement body. Please remove them.
> + }
> +
> + if (content_type == "application/octet-stream") {
'{' bracket should start on its own line.
> +
> + std::unique_ptr<GFileInfo, void(*)(void *)> full_info(g_file_query_info(file.get(), G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE,
Please break this line at 100 chars or less. That's the general standard we are trying to follow.
> + G_FILE_QUERY_INFO_NONE,
> + /* cancellable */ NULL, &error),g_object_unref);
> +
> + if (!full_info)
> + {
> + return std::string();
See comment above.
> + }
> +
> + content_type = g_file_info_get_attribute_string(full_info.get(), G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE);
This line might be longer than 100 chars as well, please break.
> + if (content_type.empty())
> + {
> + return std::string();
See comment above.
> + }
> + }
> + return content_type;
> }
>
> std::string gstreamer::Playbin::encode_uri(const std::string& uri) const
--
https://code.launchpad.net/~lorn-potter/media-hub/fix-1420728/+merge/297580
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.
More information about the Ubuntu-reviews
mailing list