Hi Howard,
On Mon, Jul 12, 2021 at 1:17 AM Yun-hao Chung <howardchung@xxxxxxxxxx> wrote:
>
> I agree this is a trick for CrOS and probably not suitable for
> upstreaming. If we want to allow/disallow profiles without
> reprobing/removing them, here is what we need to do:
> For each profile in profiles/, reject the connection if its UUID is
> not allowed. Note that checking the UUID in btd_request_authorization
> is not enough since some profiles like profiles/health/mcap.c don't
> call btd_request_authorization.
> The same check will need to be added in src/profiles.c as well so that
> we can also manage external profiles.
> Does that make sense?
Yep, afaik when Ive implemented the service plugin I did the
blocking/allowing at the service level, not sure if you want to
replicate that.
> On Fri, Jul 9, 2021 at 1:49 PM Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
> >
> > Hi Howard,
> >
> > On Wed, Jul 7, 2021 at 11:23 PM Howard Chung <howardchung@xxxxxxxxxx> wrote:
> > >
> > > When A2DP source profile is removed from adapter, a2dp_server and
> > > everything inside the object will be removed, which also releases all
> > > MediaEndpoints and MediaPlayer. When A2DP source profile is re-added,
> > > although a2dp_server will be created, clients are not able to know they
> > > can register their endpoints and player by then.
> > >
> > > This patch handles this case by unregistering Media1 interface
> > > when we remove a2dp_server, and register it back when a2dp_source is
> > > allowed.
> >
> > This sounds more like a bug fix for a regression introduced by the
> > very set, so Id recommend fixup the original patch that introduced the
> > problem, also Im afraid there could other instances like this perhaps
> > it would have been better to propagate the allow/block to the profiles
> > that way they don't have to be reprobed, I also have my doubts clients
> > would react properly to Media1 disappearing and appearing again so Id
> > leave it up if there is any endpoint/player registered to avoid having
> > them to re-register.
> >
> > >
> > > Reviewed-by: Miao-chen Chou <mcchou@xxxxxxxxxxxx>
> > > ---
> > > perform following steps
> > > 1. SetServiceAllowList to empty list
> > > 2. pair with an LE headset, then turn off the headset
> > > 3. SetServiceAllowList to "0x1234"
> > > 4. SetServiceAllowList to empty list
> > > 5. turn on the headset and check if it is reconnected.
> > >
> > > profiles/audio/a2dp.c | 2 ++
> > > profiles/audio/avrcp.c | 3 +++
> > > 2 files changed, 5 insertions(+)
> > >
> > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > > index d31ed845cbe7..26d4f365207e 100644
> > > --- a/profiles/audio/a2dp.c
> > > +++ b/profiles/audio/a2dp.c
> > > @@ -3275,6 +3275,7 @@ static int a2dp_source_server_probe(struct btd_profile *p,
> > > {
> > > struct a2dp_server *server;
> > >
> > > + media_register(adapter);
> > > DBG("path %s", adapter_get_path(adapter));
> > >
> > > server = find_server(servers, adapter);
> > > @@ -3315,6 +3316,7 @@ static void a2dp_source_server_remove(struct btd_profile *p,
> > > return;
> > >
> > > a2dp_server_unregister(server);
> > > + media_unregister(adapter);
> > > }
> > >
> > > static int a2dp_sink_server_probe(struct btd_profile *p,
> > > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> > > index ccf34b2207a9..997a5be9a0f4 100644
> > > --- a/profiles/audio/avrcp.c
> > > +++ b/profiles/audio/avrcp.c
> > > @@ -4751,6 +4751,8 @@ static void avrcp_controller_server_remove(struct btd_profile *p,
> > >
> > > if (server->tg_record_id == 0)
> > > avrcp_server_unregister(server);
> > > +
> > > + media_unregister(adapter);
> > > }
> > >
> > > static int avrcp_controller_server_probe(struct btd_profile *p,
> > > @@ -4761,6 +4763,7 @@ static int avrcp_controller_server_probe(struct btd_profile *p,
> > >
> > > DBG("path %s", adapter_get_path(adapter));
> > >
> > > + media_register(adapter);
> > > server = find_server(servers, adapter);
> > > if (server != NULL)
> > > goto done;
> > > --
> > > 2.32.0.93.g670b81a890-goog
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
