Re: [PATCH 3/3] AVRCP: Corrected metadata: Playing Time

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello, Johan

-----Original Message----- From: Johan Hedberg

Hi David,

On Sat, Aug 20, 2011, David Stockwell wrote:
Metadata item #7 should return total playing time of the track (TrackDuration)
in msec, not current position within the track.

Signed-off-by: David Stockwell <dstockwell@xxxxxxxxxxxxxxxxx>

Please remove the signed-off-by (same in the other patches)

++++++ OK, will do, and not add the line in the future. It is unnecessary, but I noticed it in other submissions, and assumed that it was becoming standard.

---
 audio/control.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/audio/control.c b/audio/control.c
index 4e10cac..047e6ac 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -196,7 +196,7 @@ enum media_info_id {
 MEDIA_INFO_TRACK = 4,
 MEDIA_INFO_N_TRACKS = 5,
 MEDIA_INFO_GENRE = 6,
- MEDIA_INFO_CURRENT_POSITION = 7,
+ MEDIA_INFO_PLAYING_TIME = 7,
 };

Would it make sense to add a MEDIA_INFO_LAST to the end of the above
list and then instead of the following:

- for (i = 1; i <= MEDIA_INFO_CURRENT_POSITION; i++) {
+ for (i = 1; i <= MEDIA_INFO_PLAYING_TIME; i++) {
 size = mp_get_media_attribute(control->mp, i,
 &pdu->params[pos]);

You'd have:

for (i = 1; i < MEDIA_INFO_LAST; i++) {

+++++ Yes, it does make sense. In fact in my "big bang" submission, that's how I did it. Will make the change and resubmit.

Seems more readable to me at least and it'd make it easier to add new
MEDIA_INFO types in the future (you only need to change the enum
definition and protect yourself against forgetting to update both
places).

+++++ I agree...absolutely. FWIW, I do have a few MEDIA_INFO items that I am sending to the BARB. Seems a waste to have a space of 4 bn possible metadata elements, and only seven used, with all the rest "reserved". Not even a space for vendor-defined elements.

Btw, it looked like this avrcp_handle_get_element_attributes function
might not be properly checking the amount of actually received data in
all necessary places before accessing the buffer (i.e. having the risk
of remotely triggered buffer overflows). Could you please verify this
and fix it if the issue really exists.

+++++ I will take a look this afternoon and either send a fix, or send a note that it looks OK.

Cheers, David

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux