After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 760382 - grl_source_resolve_sync to return correct GrlMedia
grl_source_resolve_sync to return correct GrlMedia
Status: RESOLVED OBSOLETE
Product: grilo
Classification: Other
Component: plugins
unspecified
Other All
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2016-01-09 23:02 UTC by Victor Toso
Modified: 2018-09-24 09:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
source: return the new media on _resolve_sync (2.19 KB, patch)
2016-01-09 23:02 UTC, Victor Toso
none Details | Review
tests: lua-factory on grl_source_resolve_sync() (7.81 KB, patch)
2016-01-09 23:03 UTC, Victor Toso
none Details | Review
core: check if resolved/removed media is correct (1.44 KB, patch)
2016-01-12 13:42 UTC, Juan A. Suarez Romero
reviewed Details | Review

Description Victor Toso 2016-01-09 23:02:24 UTC
source: return the new media on _resolve_sync
Comment 1 Victor Toso 2016-01-09 23:02:28 UTC
Created attachment 318611 [details] [review]
source: return the new media on _resolve_sync

grl_source_resolve_sync should return the filled GrlMedia and this could
be a different GrlMedia of the one provided as parameter.
Comment 2 Victor Toso 2016-01-09 23:03:57 UTC
Created attachment 318612 [details] [review]
tests: lua-factory on grl_source_resolve_sync()

grl_source_resolve_sync might return a different GrlMedia then the one
provieded as argument. That'll soon be true for all lua sources from
lua-factory after: https://bugzilla.gnome.org/show_bug.cgi?id=753141

So, tests handling lua sources that uses grl_source_resolve_sync()
should properly handle that.
Comment 3 Juan A. Suarez Romero 2016-01-12 13:41:42 UTC
Actually, plugins must send back the passed media with the new requested information inside, and not a new one media.

This also applies to remove(), where plugin inform back the media removed.
Comment 4 Juan A. Suarez Romero 2016-01-12 13:42:48 UTC
Created attachment 318867 [details] [review]
core: check if resolved/removed media is correct

When resolving a media (or removing), the plugin must send back the passed
media with the new requested information.

Verify that indeed the source is doing this.
Comment 5 Victor Toso 2016-01-13 09:18:32 UTC
Review of attachment 318867 [details] [review]:

::: src/grl-source.c
@@ +1984,3 @@
+    GRL_WARNING ("Source %s is resolving a different media!", grl_source_get_id (source));
+  }
+

So, this one instead of my patch or should both of them be combined?

resolve_sync returns (GrlMedia *) so I guess it should be possible or we should change it to not be possible at all.

in the case of async resolve version, I guess this could be a GRL_ERROR and we could patch the documentation to express plugin-dev to use the same GrlMedia

@@ +2399,3 @@
+  /* Verify we removed the right media */
+  if (media && media != rrc->media) {
+    GRL_WARNING ("Source %s removed a different media!", grl_source_get_id (source));

Shouldn't be an GRL_ERROR here?
Comment 6 GNOME Infrastructure Team 2018-09-24 09:39:54 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/grilo/issues/76.