[Merge] lp:~phablet-team/media-hub/fix-1498962 into lp:media-hub

Konrad Zapałowicz konrad.zapalowicz at canonical.com
Thu Jun 9 09:10:25 UTC 2016


Review: Needs Fixing code

LGTM except on the sizeof thing.

Diff comments:

> 
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp	2016-05-03 19:08:00 +0000
> +++ src/core/media/player_implementation.cpp	2016-06-08 21:03:16 +0000
> @@ -138,9 +139,19 @@
>              }
>              case Engine::State::playing:
>              {
> -                // We update the track meta data prior to updating the playback status.
> +                // We update the track metadata prior to updating the playback status.
>                  // Some MPRIS clients expect this order of events.
> -                parent->meta_data_for_current_track().set(std::get<1>(engine->track_meta_data().get()));
> +                time_t now;
> +                time(&now);
> +                char buf[sizeof "2011-10-08T07:07:09Z"];
> +                strftime(buf, sizeof buf, "%FT%TZ", gmtime(&now));

sizeof acts as a function, in this case the parenthesis should not be omitted. The line above this too.

> +                media::Track::MetaData metadata{std::get<1>(engine->track_meta_data().get())};
> +                // Setting this with second resolution makes sure that the track_meta_data property changes
> +                // and thus the track_meta_data().changed() signal gets sent out over dbus. Otherwise the
> +                // Property caching mechanism would prevent this.
> +                metadata.set_last_used(std::string{buf});
> +                update_mpris_metadata(metadata);
> +
>                  // And update our playback status.
>                  parent->playback_status().set(media::Player::playing);
>                  MH_INFO("Requesting power state");


-- 
https://code.launchpad.net/~phablet-team/media-hub/fix-1498962/+merge/296604
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.



More information about the Ubuntu-reviews mailing list